Skip to content

Commit 1483883

Browse files
jowg-amazoneirsep
jowg-amazon
and
eirsep
authored
Bug fixes for threat intel (opensearch-project#1223)
* 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>
1 parent 03e0d9b commit 1483883

File tree

11 files changed

+369
-27
lines changed

11 files changed

+369
-27
lines changed

‎src/main/java/org/opensearch/securityanalytics/threatIntel/common/TIFLockService.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ public void onFailure(final Exception e) {
7474
* @param lockModel the lock model
7575
*/
7676
public void releaseLock(final LockModel lockModel) {
77+
log.debug("Releasing lock with id [{}]", lockModel.getLockId());
7778
lockService.release(
7879
lockModel,
7980
ActionListener.wrap(released -> {}, exception -> log.error("Failed to release the lock", exception))

‎src/main/java/org/opensearch/securityanalytics/threatIntel/service/DefaultTifSourceConfigLoaderService.java‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import org.opensearch.action.support.GroupedActionListener;
77
import org.opensearch.client.Client;
88
import org.opensearch.core.action.ActionListener;
9+
import org.opensearch.index.IndexNotFoundException;
910
import org.opensearch.index.query.BoolQueryBuilder;
1011
import org.opensearch.index.query.MatchQueryBuilder;
1112
import org.opensearch.index.query.QueryBuilders;
@@ -21,6 +22,7 @@
2122
import org.opensearch.securityanalytics.threatIntel.model.SATIFSourceConfigDto;
2223
import org.opensearch.securityanalytics.threatIntel.model.TIFMetadata;
2324
import org.opensearch.securityanalytics.threatIntel.model.UrlDownloadSource;
25+
import org.opensearch.transport.RemoteTransportException;
2426

2527
import java.net.URL;
2628
import java.time.Instant;
@@ -31,8 +33,11 @@
3133
import java.util.function.Function;
3234
import java.util.stream.Collectors;
3335

36+
import static org.opensearch.securityanalytics.util.DetectorUtils.getEmptySearchResponse;
37+
3438
//todo handle refresh, update tif config
3539
// todo block creation of url based config in transport layer
40+
3641
public class DefaultTifSourceConfigLoaderService {
3742
private static final Logger log = LogManager.getLogger(DefaultTifSourceConfigLoaderService.class);
3843
private final BuiltInTIFMetadataLoader tifMetadataLoader;
@@ -64,8 +69,12 @@ public void createDefaultTifConfigsIfNotExists(ActionListener<Void> listener) {
6469
ActionListener.wrap(searchResponse -> {
6570
createTifConfigsThatDontExist(searchResponse, tifMetadataList, listener);
6671
}, e -> {
67-
log.error("Failed to search tif config index for default tif configs", e);
68-
listener.onFailure(e);
72+
if (e instanceof IndexNotFoundException || (e instanceof RemoteTransportException && e.getCause() instanceof IndexNotFoundException)) {
73+
createTifConfigsThatDontExist(getEmptySearchResponse(), tifMetadataList, listener);
74+
} else {
75+
log.error("Failed to search tif config index for default tif configs", e);
76+
listener.onFailure(e);
77+
}
6978
}));
7079
}
7180

‎src/main/java/org/opensearch/securityanalytics/threatIntel/service/SATIFSourceConfigManagementService.java‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,15 @@
3232
import org.opensearch.securityanalytics.model.STIX2IOCDto;
3333
import org.opensearch.securityanalytics.services.STIX2IOCFetchService;
3434
import org.opensearch.securityanalytics.settings.SecurityAnalyticsSettings;
35+
import org.opensearch.securityanalytics.threatIntel.common.SourceConfigType;
3536
import org.opensearch.securityanalytics.threatIntel.common.TIFJobState;
3637
import org.opensearch.securityanalytics.threatIntel.common.TIFLockService;
3738
import org.opensearch.securityanalytics.threatIntel.model.DefaultIocStoreConfig;
3839
import org.opensearch.securityanalytics.threatIntel.model.IocStoreConfig;
3940
import org.opensearch.securityanalytics.threatIntel.model.IocUploadSource;
4041
import org.opensearch.securityanalytics.threatIntel.model.SATIFSourceConfig;
4142
import org.opensearch.securityanalytics.threatIntel.model.SATIFSourceConfigDto;
43+
import org.opensearch.securityanalytics.threatIntel.model.UrlDownloadSource;
4244
import org.opensearch.securityanalytics.util.SecurityAnalyticsException;
4345

4446
import java.time.Instant;
@@ -275,6 +277,48 @@ public void updateIocAndTIFSourceConfig(
275277
try {
276278
saTifSourceConfigService.getTIFSourceConfig(saTifSourceConfigDto.getId(), ActionListener.wrap(
277279
retrievedSaTifSourceConfig -> {
280+
// 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.
281+
// 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.
282+
if (retrievedSaTifSourceConfig.isEnabledForScan() != saTifSourceConfigDto.isEnabledForScan()) {
283+
SATIFSourceConfig config = new SATIFSourceConfig(
284+
retrievedSaTifSourceConfig.getId(),
285+
retrievedSaTifSourceConfig.getVersion(),
286+
retrievedSaTifSourceConfig.getName(),
287+
retrievedSaTifSourceConfig.getFormat(),
288+
retrievedSaTifSourceConfig.getType(),
289+
retrievedSaTifSourceConfig.getDescription(),
290+
retrievedSaTifSourceConfig.getCreatedByUser(),
291+
retrievedSaTifSourceConfig.getCreatedAt(),
292+
retrievedSaTifSourceConfig.getSource(),
293+
retrievedSaTifSourceConfig.getEnabledTime(),
294+
retrievedSaTifSourceConfig.getLastUpdateTime(),
295+
retrievedSaTifSourceConfig.getSchedule(),
296+
retrievedSaTifSourceConfig.getState(),
297+
retrievedSaTifSourceConfig.getRefreshType(),
298+
Instant.now(),
299+
updatedByUser,
300+
retrievedSaTifSourceConfig.isEnabled(),
301+
retrievedSaTifSourceConfig.getIocStoreConfig(),
302+
retrievedSaTifSourceConfig.getIocTypes(),
303+
saTifSourceConfigDto.isEnabledForScan() // update only enabled_for_scan
304+
);
305+
internalUpdateTIFSourceConfig(config, ActionListener.wrap(
306+
r -> {
307+
listener.onResponse(new SATIFSourceConfigDto(r));
308+
}, e -> {
309+
String action = saTifSourceConfigDto.isEnabledForScan() ? "activate" : "deactivate";
310+
log.error(String.format("Failed to %s tif source config %s", action, retrievedSaTifSourceConfig.getId()), e);
311+
listener.onFailure(SecurityAnalyticsException.wrap(new OpenSearchException(String.format("Failed to %s tif source config %s", action, retrievedSaTifSourceConfig.getId()), e)));
312+
return;
313+
}
314+
));
315+
return;
316+
} 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
317+
log.error("Unsupported Threat intel Source Config Type passed - " + saTifSourceConfigDto.getType());
318+
listener.onFailure(new UnsupportedOperationException("Unsupported Threat intel Source Config Type passed - " + saTifSourceConfigDto.getType()));
319+
return;
320+
}
321+
278322
if (TIFJobState.AVAILABLE.equals(retrievedSaTifSourceConfig.getState()) == false && TIFJobState.REFRESH_FAILED.equals(retrievedSaTifSourceConfig.getState()) == false) {
279323
log.error("Invalid threat intel source config state. Expecting {} or {} but received {}", TIFJobState.AVAILABLE, TIFJobState.REFRESH_FAILED, retrievedSaTifSourceConfig.getState());
280324
listener.onFailure(SecurityAnalyticsException.wrap(new OpenSearchStatusException(
@@ -458,7 +502,7 @@ public void deleteTIFSourceConfig(
458502
}, e -> {
459503
log.error("Failed to get threat intel source config for [{}]", saTifSourceConfigId);
460504
if (e instanceof IndexNotFoundException) {
461-
listener.onFailure(SecurityAnalyticsException.wrap(new OpenSearchStatusException(String.format(Locale.getDefault(),"Threat intel source config [%s] not found.", saTifSourceConfigId), RestStatus.NOT_FOUND)));
505+
listener.onFailure(SecurityAnalyticsException.wrap(new OpenSearchStatusException(String.format(Locale.getDefault(), "Threat intel source config [%s] not found.", saTifSourceConfigId), RestStatus.NOT_FOUND)));
462506
} else {
463507
listener.onFailure(e);
464508
}

‎src/main/java/org/opensearch/securityanalytics/threatIntel/service/SATIFSourceConfigService.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -255,12 +255,6 @@ public void searchTIFSourceConfigs(
255255
) {
256256
SearchRequest searchRequest = getSearchRequest(searchSourceBuilder);
257257

258-
// Check to make sure the job index exists
259-
if (clusterService.state().metadata().hasIndex(SecurityAnalyticsPlugin.JOB_INDEX_NAME) == false) {
260-
actionListener.onFailure(SecurityAnalyticsException.wrap(new OpenSearchStatusException("Threat intel source config index does not exist", RestStatus.BAD_REQUEST)));
261-
return;
262-
}
263-
264258
client.search(searchRequest, ActionListener.wrap(
265259
searchResponse -> {
266260
if (searchResponse.isTimedOut()) {

‎src/main/java/org/opensearch/securityanalytics/threatIntel/transport/TransportGetIocFindingsAction.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,8 @@ protected void doExecute(Task task, GetIocFindingsRequest request, ActionListene
120120
List<String> iocIds = request.getIocIds();
121121
if (iocIds != null && !iocIds.isEmpty()) {
122122
BoolQueryBuilder iocIdQueryBuilder = QueryBuilders.boolQuery();
123-
iocIds.forEach(it -> iocIdQueryBuilder.should(QueryBuilders.matchQuery("ioc_feed_ids.ioc_id", it)));
123+
// can't use match query because it analyzes the value and considers `hyphens` as word separators
124+
iocIds.forEach(it -> iocIdQueryBuilder.should(QueryBuilders.matchPhraseQuery("ioc_feed_ids.ioc_id", it)));
124125
queryBuilder.filter(iocIdQueryBuilder);
125126
}
126127

‎src/main/java/org/opensearch/securityanalytics/threatIntel/transport/TransportIndexTIFSourceConfigAction.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,6 @@ private void retrieveLockAndCreateTIFConfig(SAIndexTIFSourceConfigRequest reques
9999
}
100100
try {
101101
SATIFSourceConfigDto saTifSourceConfigDto = request.getTIFConfigDto();
102-
if (SourceConfigType.URL_DOWNLOAD.equals(saTifSourceConfigDto.getType()) || saTifSourceConfigDto.getSource() instanceof UrlDownloadSource
103-
&& request.getMethod().equals(RestRequest.Method.POST)) {
104-
listener.onFailure(new UnsupportedOperationException("Unsupported Threat intel Source Config Type passed - " + saTifSourceConfigDto.getType()));
105-
return;
106-
}
107102
saTifSourceConfigManagementService.createOrUpdateTifSourceConfig(
108103
saTifSourceConfigDto,
109104
lock,

‎src/main/java/org/opensearch/securityanalytics/threatIntel/transport/monitor/TransportGetThreatIntelAlertsAction.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,8 @@ protected void doExecute(Task task, GetThreatIntelAlertsRequest request, ActionL
108108
SearchRequest threatIntelMonitorsSearchRequest = new SearchRequest();
109109
threatIntelMonitorsSearchRequest.indices(".opendistro-alerting-config");
110110
BoolQueryBuilder boolQueryBuilder = QueryBuilders.boolQuery();
111-
boolQueryBuilder.should().add(new BoolQueryBuilder().must(QueryBuilders.matchQuery("monitor.owner", PLUGIN_OWNER_FIELD)));
112-
boolQueryBuilder.should().add(new BoolQueryBuilder().must(QueryBuilders.matchQuery("monitor.monitor_type", ThreatIntelMonitorRunner.THREAT_INTEL_MONITOR_TYPE)));
111+
boolQueryBuilder.should().add(new BoolQueryBuilder().must(QueryBuilders.matchPhraseQuery("monitor.owner", PLUGIN_OWNER_FIELD)));
112+
boolQueryBuilder.should().add(new BoolQueryBuilder().must(QueryBuilders.matchPhraseQuery("monitor.monitor_type", ThreatIntelMonitorRunner.THREAT_INTEL_MONITOR_TYPE)));
113113
threatIntelMonitorsSearchRequest.source(new SearchSourceBuilder().query(boolQueryBuilder));
114114
transportSearchThreatIntelMonitorAction.execute(new SearchThreatIntelMonitorRequest(threatIntelMonitorsSearchRequest), ActionListener.wrap(
115115
searchResponse -> {
@@ -141,7 +141,7 @@ private void getAlerts(List<String> monitorIds,
141141
BoolQueryBuilder monitorIdMatchQuery = QueryBuilders.boolQuery();
142142
for (String monitorId : monitorIds) {
143143
monitorIdMatchQuery.should(QueryBuilders.boolQuery()
144-
.must(QueryBuilders.matchQuery("monitor_id", monitorId)));
144+
.must(QueryBuilders.matchPhraseQuery("monitor_id", monitorId)));
145145

146146
}
147147
queryBuilder.filter(monitorIdMatchQuery);

‎src/main/java/org/opensearch/securityanalytics/threatIntel/transport/monitor/TransportUpdateThreatIntelAlertStatusAction.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,8 @@ protected void doExecute(Task task, UpdateThreatIntelAlertStatusRequest request,
102102
SearchRequest threatIntelMonitorsSearchRequest = new SearchRequest();
103103
threatIntelMonitorsSearchRequest.indices(".opendistro-alerting-config");
104104
BoolQueryBuilder boolQueryBuilder = QueryBuilders.boolQuery();
105-
boolQueryBuilder.should().add(new BoolQueryBuilder().must(QueryBuilders.matchQuery("monitor.owner", PLUGIN_OWNER_FIELD)));
106-
boolQueryBuilder.should().add(new BoolQueryBuilder().must(QueryBuilders.matchQuery("monitor.monitor_type", ThreatIntelMonitorRunner.THREAT_INTEL_MONITOR_TYPE)));
105+
boolQueryBuilder.should().add(new BoolQueryBuilder().must(QueryBuilders.matchPhraseQuery("monitor.owner", PLUGIN_OWNER_FIELD)));
106+
boolQueryBuilder.should().add(new BoolQueryBuilder().must(QueryBuilders.matchPhraseQuery("monitor.monitor_type", ThreatIntelMonitorRunner.THREAT_INTEL_MONITOR_TYPE)));
107107
threatIntelMonitorsSearchRequest.source(new SearchSourceBuilder().query(boolQueryBuilder));
108108
transportSearchThreatIntelMonitorAction.execute(new SearchThreatIntelMonitorRequest(threatIntelMonitorsSearchRequest), ActionListener.wrap(
109109
searchResponse -> {
@@ -174,22 +174,22 @@ private static SearchSourceBuilder getSearchSourceQueryingForAlertsToUpdate(List
174174
BoolQueryBuilder queryBuilder = QueryBuilders.boolQuery();
175175
BoolQueryBuilder monitorIdMatchQuery = QueryBuilders.boolQuery();
176176
for (String monitorId : monitorIds) {
177-
monitorIdMatchQuery.should(QueryBuilders.matchQuery(ThreatIntelAlert.MONITOR_ID_FIELD, monitorId));
177+
monitorIdMatchQuery.should(QueryBuilders.matchPhraseQuery(ThreatIntelAlert.MONITOR_ID_FIELD, monitorId));
178178

179179
}
180180
queryBuilder.filter(monitorIdMatchQuery);
181181

182182
BoolQueryBuilder idMatchQuery = QueryBuilders.boolQuery();
183183
for (String id : request.getAlertIds()) {
184-
idMatchQuery.should(QueryBuilders.matchQuery("_id", id));
184+
idMatchQuery.should(QueryBuilders.matchPhraseQuery("_id", id));
185185

186186
}
187187
queryBuilder.filter(idMatchQuery);
188188

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

279279
}
280280
queryBuilder.filter(monitorIdMatchQuery);
281281

282282
BoolQueryBuilder idMatchQuery = QueryBuilders.boolQuery();
283283
for (String id : alertIds) {
284-
idMatchQuery.should(QueryBuilders.matchQuery("_id", id));
284+
idMatchQuery.should(QueryBuilders.matchPhraseQuery("_id", id));
285285

286286
}
287287
queryBuilder.filter(idMatchQuery);

0 commit comments

Comments
 (0)