From 63d96c3d08b6730eb1919f763e0f20f9ab8a32fc Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Wed, 22 Sep 2021 15:12:08 +0300 Subject: [PATCH 1/3] Filter out null join keys --- .../resources/additional_test_queries.toml | 27 ++++++++++++++ .../assembler/BoxedQueryRequest.java | 35 ++++++------------- 2 files changed, 37 insertions(+), 25 deletions(-) diff --git a/x-pack/plugin/eql/qa/common/src/main/resources/additional_test_queries.toml b/x-pack/plugin/eql/qa/common/src/main/resources/additional_test_queries.toml index 26a4a06354cf6..56452a74bcf8e 100644 --- a/x-pack/plugin/eql/qa/common/src/main/resources/additional_test_queries.toml +++ b/x-pack/plugin/eql/qa/common/src/main/resources/additional_test_queries.toml @@ -414,3 +414,30 @@ query = ''' process where substring(command_line, 5) regex (".*?net[1]? localgroup.*?", ".*? myappserver.py .*?") ''' + +[[queries]] +name = "sequenceOnOneNullKey" +query = ''' +sequence + [process where parent_process_path == null] by parent_process_path + [any where true] by parent_process_path +''' +expected_event_ids = [] + +[[queries]] +name = "sequenceOnTwoNullKeys" +query = ''' +sequence by ppid + [process where parent_process_path == null] by parent_process_path + [any where true] by parent_process_path +''' +expected_event_ids = [] + +[[queries]] +name = "sequenceOnImplicitNullKeys" +query = ''' +sequence by ppid, parent_process_path + [process where parent_process_path == null] + [any where true] +''' +expected_event_ids = [] diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/BoxedQueryRequest.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/BoxedQueryRequest.java index 27a5ce473ea4d..b5907ba5b4992 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/BoxedQueryRequest.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/BoxedQueryRequest.java @@ -58,6 +58,15 @@ public BoxedQueryRequest(QueryRequest original, String timestamp, List k timestampRange = rangeQuery(timestamp).timeZone("UTC").format("epoch_millis"); keys = keyNames; RuntimeUtils.addFilter(timestampRange, searchSource); + // do not join on null values + if (keyNames.isEmpty() == false) { + BoolQueryBuilder nullValuesFilter = boolQuery(); + for (int keyIndex = 0; keyIndex < keyNames.size(); keyIndex++) { + // add an "exists" query for each join key to filter out any non-existent values + nullValuesFilter.must(existsQuery(keyNames.get(keyIndex))); + } + RuntimeUtils.addFilter(nullValuesFilter, searchSource); + } } @Override @@ -112,18 +121,9 @@ public BoxedQueryRequest keys(List> values) { // iterate on all possible values for a given key newFilters = new ArrayList<>(values.size()); for (int keyIndex = 0; keyIndex < keys.size(); keyIndex++) { - - boolean hasNullValue = false; Set keyValues = new HashSet<>(BoxedQueryRequest.MAX_TERMS); - // check the given keys but make sure to double check for - // null as it translates to a different query (missing/not exists) for (List value : values) { - Object keyValue = value.get(keyIndex); - if (keyValue == null) { - hasNullValue = true; - } else { - keyValues.add(keyValue); - } + keyValues.add(value.get(keyIndex)); } // too many unique terms, don't filter on the keys @@ -141,21 +141,6 @@ public BoxedQueryRequest keys(List> values) { } else if (keyValues.size() > 1) { query = termsQuery(key, keyValues); } - - // if null values are present - // make an OR call - either terms or null/missing values - if (hasNullValue) { - BoolQueryBuilder isMissing = boolQuery().mustNot(existsQuery(key)); - if (query != null) { - query = boolQuery() - // terms query - .should(query) - // is missing - .should(isMissing); - } else { - query = isMissing; - } - } newFilters.add(query); } } From 1d66b0d69a1ece84f78087d2382c4e859a843ca2 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Thu, 23 Sep 2021 12:05:33 +0300 Subject: [PATCH 2/3] Update the EqlDataLoader with credentials (since #70114 security is enabled in this cluster). --- .../elasticsearch/xpack/eql/EqlDataLoader.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/eql/qa/correctness/src/javaRestTest/java/org/elasticsearch/xpack/eql/EqlDataLoader.java b/x-pack/plugin/eql/qa/correctness/src/javaRestTest/java/org/elasticsearch/xpack/eql/EqlDataLoader.java index b08df71faf00f..8e3526b82e126 100644 --- a/x-pack/plugin/eql/qa/correctness/src/javaRestTest/java/org/elasticsearch/xpack/eql/EqlDataLoader.java +++ b/x-pack/plugin/eql/qa/correctness/src/javaRestTest/java/org/elasticsearch/xpack/eql/EqlDataLoader.java @@ -8,6 +8,11 @@ package org.elasticsearch.xpack.eql; import org.apache.http.HttpHost; +import org.apache.http.auth.AuthScope; +import org.apache.http.auth.UsernamePasswordCredentials; +import org.apache.http.client.CredentialsProvider; +import org.apache.http.impl.client.BasicCredentialsProvider; +import org.apache.http.impl.nio.client.HttpAsyncClientBuilder; import org.apache.logging.log4j.core.config.plugins.util.PluginManager; import org.elasticsearch.action.admin.cluster.repositories.put.PutRepositoryRequest; import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequest; @@ -15,6 +20,7 @@ import org.elasticsearch.client.Request; import org.elasticsearch.client.RequestOptions; import org.elasticsearch.client.RestClient; +import org.elasticsearch.client.RestClientBuilder.HttpClientConfigCallback; import org.elasticsearch.client.RestHighLevelClient; import org.elasticsearch.client.core.CountRequest; import org.elasticsearch.common.logging.LogConfigurator; @@ -34,9 +40,16 @@ public class EqlDataLoader { public static void main(String[] args) throws IOException { // Need to setup the log configuration properly to avoid messages when creating a new RestClient PluginManager.addPackage(LogConfigurator.class.getPackage().getName()); - + final CredentialsProvider credentialsProvider = new BasicCredentialsProvider(); + credentialsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials("admin", "admin-password")); try ( RestClient client = RestClient.builder(new HttpHost("localhost", 9200)) + .setHttpClientConfigCallback(new HttpClientConfigCallback() { + @Override + public HttpAsyncClientBuilder customizeHttpClient(HttpAsyncClientBuilder httpClientBuilder) { + return httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider); + } + }) .setRequestConfigCallback( requestConfigBuilder -> requestConfigBuilder.setConnectTimeout(30000000) .setConnectionRequestTimeout(30000000) From c17502c7e1f4147ceef50c592b684159ae6e753e Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Thu, 23 Sep 2021 13:02:08 +0300 Subject: [PATCH 3/3] Address reviews --- .../correctness/src/javaRestTest/resources/queries.toml | 4 ++-- .../xpack/eql/execution/assembler/BoxedQueryRequest.java | 8 ++------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/x-pack/plugin/eql/qa/correctness/src/javaRestTest/resources/queries.toml b/x-pack/plugin/eql/qa/correctness/src/javaRestTest/resources/queries.toml index aaab73c7c4f76..a0690da0f3b7c 100644 --- a/x-pack/plugin/eql/qa/correctness/src/javaRestTest/resources/queries.toml +++ b/x-pack/plugin/eql/qa/correctness/src/javaRestTest/resources/queries.toml @@ -385,8 +385,8 @@ type = "sequence" [[queries]] queryNo = 24 -count = 1 -expected_event_ids = [2860083, 2860090, 2860098] +count = 0 +expected_event_ids = [] filter_counts = [6, 6, 6] filters = [ 'security where hostname != "newyork" and event_id == 4625', diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/BoxedQueryRequest.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/BoxedQueryRequest.java index b5907ba5b4992..f49e37ce9bf97 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/BoxedQueryRequest.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/execution/assembler/BoxedQueryRequest.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.eql.execution.assembler; -import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.RangeQueryBuilder; import org.elasticsearch.search.builder.SearchSourceBuilder; @@ -22,7 +21,6 @@ import java.util.Set; import static java.util.Collections.emptyList; -import static org.elasticsearch.index.query.QueryBuilders.boolQuery; import static org.elasticsearch.index.query.QueryBuilders.existsQuery; import static org.elasticsearch.index.query.QueryBuilders.rangeQuery; import static org.elasticsearch.index.query.QueryBuilders.termQuery; @@ -60,12 +58,10 @@ public BoxedQueryRequest(QueryRequest original, String timestamp, List k RuntimeUtils.addFilter(timestampRange, searchSource); // do not join on null values if (keyNames.isEmpty() == false) { - BoolQueryBuilder nullValuesFilter = boolQuery(); - for (int keyIndex = 0; keyIndex < keyNames.size(); keyIndex++) { + for (String keyName : keyNames) { // add an "exists" query for each join key to filter out any non-existent values - nullValuesFilter.must(existsQuery(keyNames.get(keyIndex))); + RuntimeUtils.addFilter(existsQuery(keyName), searchSource); } - RuntimeUtils.addFilter(nullValuesFilter, searchSource); } }