Skip to content

Commit

Permalink
Bug fixes for threat intel (opensearch-project#1223)
Browse files Browse the repository at this point in the history 
* fix default source config behavior and release lock

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

* add check for multinode index not exist

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

* add tests

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

* replace match query with match phrase query in threat intel to avoid tokenization/analysis of value

Signed-off-by: Surya Sashank Nistala <snistala@amazon.com>

* added test to verify deactiavte ioc_upload

Signed-off-by: Surya Sashank Nistala <snistala@amazon.com>

* add debug log to release lock

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

* fix test

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

* fix update url download and add test

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

---------

Signed-off-by: Joanne Wang <jowg@amazon.com>
Signed-off-by: Surya Sashank Nistala <snistala@amazon.com>
Co-authored-by: Surya Sashank Nistala <snistala@amazon.com>
  • Loading branch information
@jowg-amazon @eirsep
jowg-amazon and eirsep authored Aug 8, 2024
1 parent 03e0d9b commit 1483883
Show file tree
Hide file tree
Showing 11 changed files with 369 additions and 27 deletions.
1 change: 1 addition & 0 deletions src/main/java/org/opensearch/securityanalytics/threatIntel/common/TIFLockService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ public void onFailure(final Exception e) {
* @param lockModel the lock model
*/
public void releaseLock(final LockModel lockModel) {
log.debug("Releasing lock with id [{}]", lockModel.getLockId());
lockService.release(
lockModel,
ActionListener.wrap(released -> {}, exception -> log.error("Failed to release the lock", exception))
Expand Down
13 changes: 11 additions & 2 deletions ...opensearch/securityanalytics/threatIntel/service/DefaultTifSourceConfigLoaderService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import org.opensearch.action.support.GroupedActionListener;
import org.opensearch.client.Client;
import org.opensearch.core.action.ActionListener;
import org.opensearch.index.IndexNotFoundException;
import org.opensearch.index.query.BoolQueryBuilder;
import org.opensearch.index.query.MatchQueryBuilder;
import org.opensearch.index.query.QueryBuilders;
Expand All @@ -21,6 +22,7 @@
import org.opensearch.securityanalytics.threatIntel.model.SATIFSourceConfigDto;
import org.opensearch.securityanalytics.threatIntel.model.TIFMetadata;
import org.opensearch.securityanalytics.threatIntel.model.UrlDownloadSource;
import org.opensearch.transport.RemoteTransportException;

import java.net.URL;
import java.time.Instant;
Expand All @@ -31,8 +33,11 @@
import java.util.function.Function;
import java.util.stream.Collectors;

import static org.opensearch.securityanalytics.util.DetectorUtils.getEmptySearchResponse;

//todo handle refresh, update tif config
// todo block creation of url based config in transport layer

public class DefaultTifSourceConfigLoaderService {
private static final Logger log = LogManager.getLogger(DefaultTifSourceConfigLoaderService.class);
private final BuiltInTIFMetadataLoader tifMetadataLoader;
Expand Down Expand Up @@ -64,8 +69,12 @@ public void createDefaultTifConfigsIfNotExists(ActionListener<Void> listener) {
ActionListener.wrap(searchResponse -> {
createTifConfigsThatDontExist(searchResponse, tifMetadataList, listener);
}, e -> {
log.error("Failed to search tif config index for default tif configs", e);
listener.onFailure(e);
if (e instanceof IndexNotFoundException || (e instanceof RemoteTransportException && e.getCause() instanceof IndexNotFoundException)) {
createTifConfigsThatDontExist(getEmptySearchResponse(), tifMetadataList, listener);
} else {
log.error("Failed to search tif config index for default tif configs", e);
listener.onFailure(e);
}
}));
}

Expand Down
46 changes: 45 additions & 1 deletion .../opensearch/securityanalytics/threatIntel/service/SATIFSourceConfigManagementService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,15 @@
import org.opensearch.securityanalytics.model.STIX2IOCDto;
import org.opensearch.securityanalytics.services.STIX2IOCFetchService;
import org.opensearch.securityanalytics.settings.SecurityAnalyticsSettings;
import org.opensearch.securityanalytics.threatIntel.common.SourceConfigType;
import org.opensearch.securityanalytics.threatIntel.common.TIFJobState;
import org.opensearch.securityanalytics.threatIntel.common.TIFLockService;
import org.opensearch.securityanalytics.threatIntel.model.DefaultIocStoreConfig;
import org.opensearch.securityanalytics.threatIntel.model.IocStoreConfig;
import org.opensearch.securityanalytics.threatIntel.model.IocUploadSource;
import org.opensearch.securityanalytics.threatIntel.model.SATIFSourceConfig;
import org.opensearch.securityanalytics.threatIntel.model.SATIFSourceConfigDto;
import org.opensearch.securityanalytics.threatIntel.model.UrlDownloadSource;
import org.opensearch.securityanalytics.util.SecurityAnalyticsException;

import java.time.Instant;
Expand Down Expand Up @@ -275,6 +277,48 @@ public void updateIocAndTIFSourceConfig(
try {
saTifSourceConfigService.getTIFSourceConfig(saTifSourceConfigDto.getId(), ActionListener.wrap(
retrievedSaTifSourceConfig -> {
// Due to the lack of a different API to do activate/deactivate we will check if enabled_for_scan variable is changed between model and request.
// If yes, we will ONLY update enabled_for_scan field and ignore any updates to the rest of the fields to simulate a dedicated activate/deactivate API.
if (retrievedSaTifSourceConfig.isEnabledForScan() != saTifSourceConfigDto.isEnabledForScan()) {
SATIFSourceConfig config = new SATIFSourceConfig(
retrievedSaTifSourceConfig.getId(),
retrievedSaTifSourceConfig.getVersion(),
retrievedSaTifSourceConfig.getName(),
retrievedSaTifSourceConfig.getFormat(),
retrievedSaTifSourceConfig.getType(),
retrievedSaTifSourceConfig.getDescription(),
retrievedSaTifSourceConfig.getCreatedByUser(),
retrievedSaTifSourceConfig.getCreatedAt(),
retrievedSaTifSourceConfig.getSource(),
retrievedSaTifSourceConfig.getEnabledTime(),
retrievedSaTifSourceConfig.getLastUpdateTime(),
retrievedSaTifSourceConfig.getSchedule(),
retrievedSaTifSourceConfig.getState(),
retrievedSaTifSourceConfig.getRefreshType(),
Instant.now(),
updatedByUser,
retrievedSaTifSourceConfig.isEnabled(),
retrievedSaTifSourceConfig.getIocStoreConfig(),
retrievedSaTifSourceConfig.getIocTypes(),
saTifSourceConfigDto.isEnabledForScan() // update only enabled_for_scan
);
internalUpdateTIFSourceConfig(config, ActionListener.wrap(
r -> {
listener.onResponse(new SATIFSourceConfigDto(r));
}, e -> {
String action = saTifSourceConfigDto.isEnabledForScan() ? "activate" : "deactivate";
log.error(String.format("Failed to %s tif source config %s", action, retrievedSaTifSourceConfig.getId()), e);
listener.onFailure(SecurityAnalyticsException.wrap(new OpenSearchException(String.format("Failed to %s tif source config %s", action, retrievedSaTifSourceConfig.getId()), e)));
return;
}
));
return;
} else if (SourceConfigType.URL_DOWNLOAD.equals(saTifSourceConfigDto.getType()) || saTifSourceConfigDto.getSource() instanceof UrlDownloadSource) { // fail if enabled_for_scan isn't changed and type is url download
log.error("Unsupported Threat intel Source Config Type passed - " + saTifSourceConfigDto.getType());
listener.onFailure(new UnsupportedOperationException("Unsupported Threat intel Source Config Type passed - " + saTifSourceConfigDto.getType()));
return;
}

if (TIFJobState.AVAILABLE.equals(retrievedSaTifSourceConfig.getState()) == false && TIFJobState.REFRESH_FAILED.equals(retrievedSaTifSourceConfig.getState()) == false) {
log.error("Invalid threat intel source config state. Expecting {} or {} but received {}", TIFJobState.AVAILABLE, TIFJobState.REFRESH_FAILED, retrievedSaTifSourceConfig.getState());
listener.onFailure(SecurityAnalyticsException.wrap(new OpenSearchStatusException(
Expand Down Expand Up @@ -458,7 +502,7 @@ public void deleteTIFSourceConfig(
}, e -> {
log.error("Failed to get threat intel source config for [{}]", saTifSourceConfigId);
if (e instanceof IndexNotFoundException) {
listener.onFailure(SecurityAnalyticsException.wrap(new OpenSearchStatusException(String.format(Locale.getDefault(),"Threat intel source config [%s] not found.", saTifSourceConfigId), RestStatus.NOT_FOUND)));
listener.onFailure(SecurityAnalyticsException.wrap(new OpenSearchStatusException(String.format(Locale.getDefault(), "Threat intel source config [%s] not found.", saTifSourceConfigId), RestStatus.NOT_FOUND)));
} else {
listener.onFailure(e);
}
Expand Down
6 changes: 0 additions & 6 deletions ...n/java/org/opensearch/securityanalytics/threatIntel/service/SATIFSourceConfigService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,12 +255,6 @@ public void searchTIFSourceConfigs(
) {
SearchRequest searchRequest = getSearchRequest(searchSourceBuilder);

// Check to make sure the job index exists
if (clusterService.state().metadata().hasIndex(SecurityAnalyticsPlugin.JOB_INDEX_NAME) == false) {
actionListener.onFailure(SecurityAnalyticsException.wrap(new OpenSearchStatusException("Threat intel source config index does not exist", RestStatus.BAD_REQUEST)));
return;
}

client.search(searchRequest, ActionListener.wrap(
searchResponse -> {
if (searchResponse.isTimedOut()) {
Expand Down
3 changes: 2 additions & 1 deletion ...org/opensearch/securityanalytics/threatIntel/transport/TransportGetIocFindingsAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ protected void doExecute(Task task, GetIocFindingsRequest request, ActionListene
List<String> iocIds = request.getIocIds();
if (iocIds != null && !iocIds.isEmpty()) {
BoolQueryBuilder iocIdQueryBuilder = QueryBuilders.boolQuery();
iocIds.forEach(it -> iocIdQueryBuilder.should(QueryBuilders.matchQuery("ioc_feed_ids.ioc_id", it)));
// can't use match query because it analyzes the value and considers `hyphens` as word separators
iocIds.forEach(it -> iocIdQueryBuilder.should(QueryBuilders.matchPhraseQuery("ioc_feed_ids.ioc_id", it)));
queryBuilder.filter(iocIdQueryBuilder);
}

Expand Down
5 changes: 0 additions & 5 deletions ...ensearch/securityanalytics/threatIntel/transport/TransportIndexTIFSourceConfigAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,6 @@ private void retrieveLockAndCreateTIFConfig(SAIndexTIFSourceConfigRequest reques
}
try {
SATIFSourceConfigDto saTifSourceConfigDto = request.getTIFConfigDto();
if (SourceConfigType.URL_DOWNLOAD.equals(saTifSourceConfigDto.getType()) || saTifSourceConfigDto.getSource() instanceof UrlDownloadSource
&& request.getMethod().equals(RestRequest.Method.POST)) {
listener.onFailure(new UnsupportedOperationException("Unsupported Threat intel Source Config Type passed - " + saTifSourceConfigDto.getType()));
return;
}
saTifSourceConfigManagementService.createOrUpdateTifSourceConfig(
saTifSourceConfigDto,
lock,
Expand Down
6 changes: 3 additions & 3 deletions .../securityanalytics/threatIntel/transport/monitor/TransportGetThreatIntelAlertsAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ protected void doExecute(Task task, GetThreatIntelAlertsRequest request, ActionL
SearchRequest threatIntelMonitorsSearchRequest = new SearchRequest();
threatIntelMonitorsSearchRequest.indices(".opendistro-alerting-config");
BoolQueryBuilder boolQueryBuilder = QueryBuilders.boolQuery();
boolQueryBuilder.should().add(new BoolQueryBuilder().must(QueryBuilders.matchQuery("monitor.owner", PLUGIN_OWNER_FIELD)));
boolQueryBuilder.should().add(new BoolQueryBuilder().must(QueryBuilders.matchQuery("monitor.monitor_type", ThreatIntelMonitorRunner.THREAT_INTEL_MONITOR_TYPE)));
boolQueryBuilder.should().add(new BoolQueryBuilder().must(QueryBuilders.matchPhraseQuery("monitor.owner", PLUGIN_OWNER_FIELD)));
boolQueryBuilder.should().add(new BoolQueryBuilder().must(QueryBuilders.matchPhraseQuery("monitor.monitor_type", ThreatIntelMonitorRunner.THREAT_INTEL_MONITOR_TYPE)));
threatIntelMonitorsSearchRequest.source(new SearchSourceBuilder().query(boolQueryBuilder));
transportSearchThreatIntelMonitorAction.execute(new SearchThreatIntelMonitorRequest(threatIntelMonitorsSearchRequest), ActionListener.wrap(
searchResponse -> {
Expand Down Expand Up @@ -141,7 +141,7 @@ private void getAlerts(List<String> monitorIds,
BoolQueryBuilder monitorIdMatchQuery = QueryBuilders.boolQuery();
for (String monitorId : monitorIds) {
monitorIdMatchQuery.should(QueryBuilders.boolQuery()
.must(QueryBuilders.matchQuery("monitor_id", monitorId)));
.must(QueryBuilders.matchPhraseQuery("monitor_id", monitorId)));

}
queryBuilder.filter(monitorIdMatchQuery);
Expand Down
16 changes: 8 additions & 8 deletions ...yanalytics/threatIntel/transport/monitor/TransportUpdateThreatIntelAlertStatusAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ protected void doExecute(Task task, UpdateThreatIntelAlertStatusRequest request,
SearchRequest threatIntelMonitorsSearchRequest = new SearchRequest();
threatIntelMonitorsSearchRequest.indices(".opendistro-alerting-config");
BoolQueryBuilder boolQueryBuilder = QueryBuilders.boolQuery();
boolQueryBuilder.should().add(new BoolQueryBuilder().must(QueryBuilders.matchQuery("monitor.owner", PLUGIN_OWNER_FIELD)));
boolQueryBuilder.should().add(new BoolQueryBuilder().must(QueryBuilders.matchQuery("monitor.monitor_type", ThreatIntelMonitorRunner.THREAT_INTEL_MONITOR_TYPE)));
boolQueryBuilder.should().add(new BoolQueryBuilder().must(QueryBuilders.matchPhraseQuery("monitor.owner", PLUGIN_OWNER_FIELD)));
boolQueryBuilder.should().add(new BoolQueryBuilder().must(QueryBuilders.matchPhraseQuery("monitor.monitor_type", ThreatIntelMonitorRunner.THREAT_INTEL_MONITOR_TYPE)));
threatIntelMonitorsSearchRequest.source(new SearchSourceBuilder().query(boolQueryBuilder));
transportSearchThreatIntelMonitorAction.execute(new SearchThreatIntelMonitorRequest(threatIntelMonitorsSearchRequest), ActionListener.wrap(
searchResponse -> {
Expand Down Expand Up @@ -174,22 +174,22 @@ private static SearchSourceBuilder getSearchSourceQueryingForAlertsToUpdate(List
BoolQueryBuilder queryBuilder = QueryBuilders.boolQuery();
BoolQueryBuilder monitorIdMatchQuery = QueryBuilders.boolQuery();
for (String monitorId : monitorIds) {
monitorIdMatchQuery.should(QueryBuilders.matchQuery(ThreatIntelAlert.MONITOR_ID_FIELD, monitorId));
monitorIdMatchQuery.should(QueryBuilders.matchPhraseQuery(ThreatIntelAlert.MONITOR_ID_FIELD, monitorId));

}
queryBuilder.filter(monitorIdMatchQuery);

BoolQueryBuilder idMatchQuery = QueryBuilders.boolQuery();
for (String id : request.getAlertIds()) {
idMatchQuery.should(QueryBuilders.matchQuery("_id", id));
idMatchQuery.should(QueryBuilders.matchPhraseQuery("_id", id));

}
queryBuilder.filter(idMatchQuery);

if (request.getState() == Alert.State.COMPLETED) {
queryBuilder.filter(QueryBuilders.matchQuery(ThreatIntelAlert.STATE_FIELD, Alert.State.ACKNOWLEDGED.toString()));
queryBuilder.filter(QueryBuilders.matchPhraseQuery(ThreatIntelAlert.STATE_FIELD, Alert.State.ACKNOWLEDGED.toString()));
} else if (request.getState() == Alert.State.ACKNOWLEDGED) {
queryBuilder.filter(QueryBuilders.matchQuery(ThreatIntelAlert.STATE_FIELD, Alert.State.ACTIVE.toString()));
queryBuilder.filter(QueryBuilders.matchPhraseQuery(ThreatIntelAlert.STATE_FIELD, Alert.State.ACTIVE.toString()));
} else {
log.error("Threat intel monitor not found. No alerts to update");
listener.onFailure(new SecurityAnalyticsException("Threat intel monitor not found. No alerts to update",
Expand Down Expand Up @@ -274,14 +274,14 @@ private static SearchSourceBuilder getSearchSourceQueryingForUpdatedAlerts(List<
BoolQueryBuilder queryBuilder = QueryBuilders.boolQuery();
BoolQueryBuilder monitorIdMatchQuery = QueryBuilders.boolQuery();
for (String monitorId : monitorIds) {
monitorIdMatchQuery.should(QueryBuilders.matchQuery(ThreatIntelAlert.MONITOR_ID_FIELD, monitorId));
monitorIdMatchQuery.should(QueryBuilders.matchPhraseQuery(ThreatIntelAlert.MONITOR_ID_FIELD, monitorId));

}
queryBuilder.filter(monitorIdMatchQuery);

BoolQueryBuilder idMatchQuery = QueryBuilders.boolQuery();
for (String id : alertIds) {
idMatchQuery.should(QueryBuilders.matchQuery("_id", id));
idMatchQuery.should(QueryBuilders.matchPhraseQuery("_id", id));

}
queryBuilder.filter(idMatchQuery);
Expand Down
Loading

0 comments on commit 1483883

Please sign in to comment.