Skip to content

Commit

Permalink
Fix duplicate ecs mappings which returns incorrect log index field in…
Browse files Browse the repository at this point in the history 
… mapping view API (opensearch-project#786) (opensearch-project#788)

* field mapping changes

Signed-off-by: Joanne Wang <jowg@amazon.com>

* add integ test

Signed-off-by: Joanne Wang <jowg@amazon.com>

* turn unmappedfieldaliases as set and add integ test

Signed-off-by: Joanne Wang <jowg@amazon.com>

* add comments

Signed-off-by: Joanne Wang <jowg@amazon.com>

* fix integ tests

Signed-off-by: Joanne Wang <jowg@amazon.com>

* moved logic to method for better readability

Signed-off-by: Joanne Wang <jowg@amazon.com>

---------

Signed-off-by: Joanne Wang <jowg@amazon.com>
  • Loading branch information
@jowg-amazon
jowg-amazon committed Mar 14, 2024
1 parent aa4706d commit 8e63d89
Show file tree
Hide file tree
Showing 3 changed files with 238 additions and 20 deletions.
83 changes: 65 additions & 18 deletions src/main/java/org/opensearch/securityanalytics/mapper/MapperService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
import org.opensearch.cluster.metadata.MappingMetadata;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.xcontent.XContentFactory;
import org.opensearch.core.action.ActionListener;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.securityanalytics.action.GetIndexMappingsResponse;
Expand All @@ -43,6 +45,16 @@
import org.opensearch.securityanalytics.util.IndexUtils;
import org.opensearch.securityanalytics.util.SecurityAnalyticsException;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

import static org.opensearch.securityanalytics.mapper.MapperUtils.PATH;
import static org.opensearch.securityanalytics.mapper.MapperUtils.PROPERTIES;
Expand Down Expand Up @@ -80,6 +92,7 @@ public void createMappingAction(String indexName, String logType, String aliasMa
if (IndexUtils.isDataStream(indexName, this.clusterService.state())) {
String writeIndex = IndexUtils.getWriteIndex(indexName, this.clusterService.state());
if (writeIndex != null) {
log.debug("Write index for {} is {}", indexName, writeIndex);
index = writeIndex;
}
}
Expand All @@ -91,6 +104,7 @@ public void onResponse(GetMappingsResponse getMappingsResponse) {
applyAliasMappings(getMappingsResponse.getMappings(), logType, aliasMappings, partial, new ActionListener<>() {
@Override
public void onResponse(Collection<CreateMappingResult> createMappingResponse) {
log.debug("Completed create mappings for {}", indexName);
// We will return ack==false if one of the requests returned that
// else return ack==true
Optional<AcknowledgedResponse> notAckd = createMappingResponse.stream()
Expand All @@ -109,6 +123,7 @@ public void onResponse(Collection<CreateMappingResult> createMappingResponse) {

@Override
public void onFailure(Exception e) {
log.debug("Failed to create mappings for {}", indexName );
actionListener.onFailure(e);
}
});
Expand All @@ -122,7 +137,7 @@ public void onFailure(Exception e) {
}

private void applyAliasMappings(Map<String, MappingMetadata> indexMappings, String logType, String aliasMappings, boolean partial, ActionListener<Collection<CreateMappingResult>> actionListener) {
int numOfIndices = indexMappings.size();
int numOfIndices = indexMappings.size();

GroupedActionListener doCreateMappingActionsListener = new GroupedActionListener(new ActionListener<Collection<CreateMappingResult>>() {
@Override
Expand Down Expand Up @@ -150,12 +165,13 @@ public void onFailure(Exception e) {

/**
* Applies alias mappings to index.
* @param indexName Index name
*
* @param indexName Index name
* @param mappingMetadata Index mappings
* @param logType Rule topic spcifying specific alias templates
* @param aliasMappings User-supplied alias mappings
* @param partial Partial flag indicating if we should apply mappings partially, in case source index doesn't have all paths specified in alias mappings
* @param actionListener actionListener used to return response/error
* @param logType Rule topic spcifying specific alias templates
* @param aliasMappings User-supplied alias mappings
* @param partial Partial flag indicating if we should apply mappings partially, in case source index doesn't have all paths specified in alias mappings
* @param actionListener actionListener used to return response/error
*/
private void doCreateMapping(
String indexName,
Expand Down Expand Up @@ -450,9 +466,10 @@ public void onFailure(Exception e) {

/**
* Constructs Mappings View of index
* @param logType Log Type
*
* @param logType Log Type
* @param actionListener Action Listener
* @param concreteIndex Concrete Index name for which we're computing Mappings View
* @param concreteIndex Concrete Index name for which we're computing Mappings View
*/
private void doGetMappingsView(String logType, ActionListener<GetMappingsViewResponse> actionListener, String concreteIndex) {
GetMappingsRequest getMappingsRequest = new GetMappingsRequest().indices(concreteIndex);
Expand All @@ -477,17 +494,20 @@ public void onResponse(GetMappingsResponse getMappingsResponse) {
String rawPath = requiredField.getRawField();
String ocsfPath = requiredField.getOcsf();
if (allFieldsFromIndex.contains(rawPath)) {
if (alias != null) {
// Maintain list of found paths in index
applyableAliases.add(alias);
} else {
applyableAliases.add(rawPath);
// if the alias was already added into applyable aliases, then skip to avoid duplicates
if (!applyableAliases.contains(alias) && !applyableAliases.contains(rawPath)) {
if (alias != null) {
// Maintain list of found paths in index
applyableAliases.add(alias);
} else {
applyableAliases.add(rawPath);
}
pathsOfApplyableAliases.add(rawPath);
}
pathsOfApplyableAliases.add(rawPath);
} else if (allFieldsFromIndex.contains(ocsfPath)) {
applyableAliases.add(alias);
pathsOfApplyableAliases.add(ocsfPath);
} else if ((alias == null && allFieldsFromIndex.contains(rawPath) == false) || allFieldsFromIndex.contains(alias) == false) {
} else if ((alias == null && allFieldsFromIndex.contains(rawPath) == false) || allFieldsFromIndex.contains(alias) == false) {
if (alias != null) {
// we don't want to send back aliases which have same name as existing field in index
unmappedFieldAliases.add(alias);
Expand All @@ -497,13 +517,21 @@ public void onResponse(GetMappingsResponse getMappingsResponse) {
}
}

// turn unmappedFieldAliases into a set to remove duplicates
Set<String> setOfUnmappedFieldAliases = new HashSet<>(unmappedFieldAliases);

// filter out aliases that were included in applyableAliases already
List<String> filteredUnmappedFieldAliases = setOfUnmappedFieldAliases.stream()
.filter(e -> false == applyableAliases.contains(e))
.collect(Collectors.toList());

Map<String, Map<String, String>> aliasMappingFields = new HashMap<>();
XContentBuilder aliasMappingsObj = XContentFactory.jsonBuilder().startObject();
for (LogType.Mapping mapping: requiredFields) {
if (allFieldsFromIndex.contains(mapping.getOcsf())) {
aliasMappingFields.put(mapping.getEcs(), Map.of("type", "alias", "path", mapping.getOcsf()));
} else if (mapping.getEcs() != null) {
aliasMappingFields.put(mapping.getEcs(), Map.of("type", "alias", "path", mapping.getRawField()));
shouldUpdateEcsMappingAndMaybeUpdates(mapping, aliasMappingFields, pathsOfApplyableAliases);
} else if (mapping.getEcs() == null) {
aliasMappingFields.put(mapping.getRawField(), Map.of("type", "alias", "path", mapping.getRawField()));
}
Expand All @@ -518,9 +546,8 @@ public void onResponse(GetMappingsResponse getMappingsResponse) {
.stream()
.filter(e -> pathsOfApplyableAliases.contains(e) == false)
.collect(Collectors.toList());

actionListener.onResponse(
new GetMappingsViewResponse(aliasMappings, unmappedIndexFields, unmappedFieldAliases)
new GetMappingsViewResponse(aliasMappings, unmappedIndexFields, filteredUnmappedFieldAliases)
);
} catch (Exception e) {
actionListener.onFailure(e);
Expand All @@ -534,6 +561,26 @@ public void onFailure(Exception e) {
});
}

/**
* Only updates the alias mapping fields if the ecs key has not been mapped yet
* or if pathOfApplyableAliases contains the raw field
*
* @param mapping
* @param aliasMappingFields
* @param pathsOfApplyableAliases
*/
private static void shouldUpdateEcsMappingAndMaybeUpdates(LogType.Mapping mapping, Map<String, Map<String, String>> aliasMappingFields, List<String> pathsOfApplyableAliases) {
// check if aliasMappingFields already contains a key
if (aliasMappingFields.containsKey(mapping.getEcs())) {
// if the pathOfApplyableAliases contains the raw field, then override the existing map
if (pathsOfApplyableAliases.contains(mapping.getRawField())) {
aliasMappingFields.put(mapping.getEcs(), Map.of("type", "alias", "path", mapping.getRawField()));
}
} else {
aliasMappingFields.put(mapping.getEcs(), Map.of("type", "alias", "path", mapping.getRawField()));
}
}

/**
* Given index name, resolves it to single concrete index, depending on what initial <code>indexName</code> is.
* In case of Datastream or Alias, WriteIndex would be returned. In case of index pattern, newest index by creation date would be returned.
Expand Down
171 changes: 171 additions & 0 deletions src/test/java/org/opensearch/securityanalytics/mapper/MapperRestApiIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,114 @@ public void testGetMappingsViewLinuxSuccess() throws IOException {
assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
}

// Tests mappings where multiple raw fields correspond to one ecs value
public void testGetMappingsViewWindowsSuccess() throws IOException {

String testIndexName = "get_mappings_view_index";

createSampleWindex(testIndexName);

// Execute GetMappingsViewAction to add alias mapping for index
Request request = new Request("GET", SecurityAnalyticsPlugin.MAPPINGS_VIEW_BASE_URI);
// both req params and req body are supported
request.addParameter("index_name", testIndexName);
request.addParameter("rule_topic", "windows");
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");
assertEquals(3, props.size());
assertTrue(props.containsKey("winlog.event_data.LogonType"));
assertTrue(props.containsKey("winlog.provider_name"));
assertTrue(props.containsKey("host.hostname"));

// Verify unmapped index fields
List<String> unmappedIndexFields = (List<String>) respMap.get("unmapped_index_fields");
assertEquals(3, unmappedIndexFields.size());
assert(unmappedIndexFields.contains("plain1"));
assert(unmappedIndexFields.contains("ParentUser.first"));
assert(unmappedIndexFields.contains("ParentUser.last"));

// Verify unmapped field aliases
List<String> filteredUnmappedFieldAliases = (List<String>) respMap.get("unmapped_field_aliases");
assertEquals(191, filteredUnmappedFieldAliases.size());
assert(!filteredUnmappedFieldAliases.contains("winlog.event_data.LogonType"));
assert(!filteredUnmappedFieldAliases.contains("winlog.provider_name"));
assert(!filteredUnmappedFieldAliases.contains("host.hostname"));
List<HashMap<String, Object>> iocFieldsList = (List<HashMap<String, Object>>) respMap.get(GetMappingsViewResponse.THREAT_INTEL_FIELD_ALIASES);
assertEquals(iocFieldsList.size(), 1);

// Index a doc for a field with multiple raw fields corresponding to one ecs field
indexDoc(testIndexName, "1", "{ \"EventID\": 1 }");
// Execute GetMappingsViewAction to add alias mapping for index
request = new Request("GET", SecurityAnalyticsPlugin.MAPPINGS_VIEW_BASE_URI);
// both req params and req body are supported
request.addParameter("index_name", testIndexName);
request.addParameter("rule_topic", "windows");
response = client().performRequest(request);
assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
respMap = responseAsMap(response);

// Verify alias mappings
props = (Map<String, Object>) respMap.get("properties");
assertEquals(4, props.size());
assertTrue(props.containsKey("winlog.event_id"));

// verify unmapped index fields
unmappedIndexFields = (List<String>) respMap.get("unmapped_index_fields");
assertEquals(3, unmappedIndexFields.size());

// verify unmapped field aliases
filteredUnmappedFieldAliases = (List<String>) respMap.get("unmapped_field_aliases");
assertEquals(190, filteredUnmappedFieldAliases.size());
assert(!filteredUnmappedFieldAliases.contains("winlog.event_id"));
}

// Tests mappings where multiple raw fields correspond to one ecs value and all fields are present in the index
public void testGetMappingsViewMulitpleRawFieldsSuccess() throws IOException {

String testIndexName = "get_mappings_view_index";

createSampleWindex(testIndexName);
String sampleDoc = "{" +
" \"EventID\": 1," +
" \"EventId\": 2," +
" \"event_uid\": 3" +
"}";
indexDoc(testIndexName, "1", sampleDoc);

// Execute GetMappingsViewAction to add alias mapping for index
Request request = new Request("GET", SecurityAnalyticsPlugin.MAPPINGS_VIEW_BASE_URI);
// both req params and req body are supported
request.addParameter("index_name", testIndexName);
request.addParameter("rule_topic", "windows");
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");
assertEquals(4, props.size());
assertTrue(props.containsKey("winlog.event_data.LogonType"));
assertTrue(props.containsKey("winlog.provider_name"));
assertTrue(props.containsKey("host.hostname"));
assertTrue(props.containsKey("winlog.event_id"));

// Verify unmapped index fields
List<String> unmappedIndexFields = (List<String>) respMap.get("unmapped_index_fields");
assertEquals(5, unmappedIndexFields.size());

// Verify unmapped field aliases
List<String> filteredUnmappedFieldAliases = (List<String>) respMap.get("unmapped_field_aliases");
assertEquals(190, filteredUnmappedFieldAliases.size());
assert(!filteredUnmappedFieldAliases.contains("winlog.event_data.LogonType"));
assert(!filteredUnmappedFieldAliases.contains("winlog.provider_name"));
assert(!filteredUnmappedFieldAliases.contains("host.hostname"));
assert(!filteredUnmappedFieldAliases.contains("winlog.event_id"));
}

public void testCreateMappings_withDatastream_success() throws IOException {
String datastream = "test_datastream";

Expand Down Expand Up @@ -1273,6 +1381,69 @@ private void createSampleIndex(String indexName, Settings settings, String alias
assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
}

private void createSampleWindex(String indexName) throws IOException {
createSampleWindex(indexName, Settings.EMPTY, null);
}

private void createSampleWindex(String indexName, Settings settings, String aliases) throws IOException {
String indexMapping =
" \"properties\": {" +
" \"LogonType\": {" +
" \"type\": \"integer\"" +
" }," +
" \"Provider\": {" +
" \"type\": \"text\"" +
" }," +
" \"hostname\": {" +
" \"type\": \"text\"" +
" }," +
" \"plain1\": {" +
" \"type\": \"integer\"" +
" }," +
" \"ParentUser\":{" +
" \"type\":\"nested\"," +
" \"properties\":{" +
" \"first\":{" +
" \"type\":\"text\"," +
" \"fields\":{" +
" \"keyword\":{" +
" \"type\":\"keyword\"," +
" \"ignore_above\":256" +
"}" +
"}" +
"}," +
" \"last\":{" +
"\"type\":\"text\"," +
"\"fields\":{" +
" \"keyword\":{" +
" \"type\":\"keyword\"," +
" \"ignore_above\":256" +
"}" +
"}" +
"}" +
"}" +
"}" +
" }";

createIndex(indexName, settings, indexMapping, aliases);

// Insert sample doc with event_uid not explicitly mapped
String sampleDoc = "{" +
" \"LogonType\":1," +
" \"Provider\":\"Microsoft-Windows-Security-Auditing\"," +
" \"hostname\":\"FLUXCAPACITOR\"" +
"}";

// Index doc
Request indexRequest = new Request("POST", indexName + "/_doc?refresh=wait_for");
indexRequest.setJsonEntity(sampleDoc);
Response response = client().performRequest(indexRequest);
assertEquals(HttpStatus.SC_CREATED, response.getStatusLine().getStatusCode());
// Refresh everything
response = client().performRequest(new Request("POST", "_refresh"));
assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
}

private void createSampleDatastream(String datastreamName) throws IOException {
String indexMapping =
" \"properties\": {" +
Expand Down
Loading

0 comments on commit 8e63d89

Please sign in to comment.