Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
import org.elasticsearch.client.RestHighLevelClient;
import org.elasticsearch.client.eql.EqlSearchRequest;
import org.elasticsearch.client.eql.EqlSearchResponse;
import org.elasticsearch.client.indices.CreateIndexRequest;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.Streams;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.common.xcontent.json.JsonXContent;
Expand Down Expand Up @@ -55,6 +57,12 @@ private static void setupData(CommonEqlActionTestCase tc) throws Exception {
return;
}

CreateIndexRequest request = new CreateIndexRequest(testIndexName)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed mapping to IP type, this creating the index with mapping.

.mapping(Streams.readFully(CommonEqlActionTestCase.class.getResourceAsStream("/mapping-default.json")),
XContentType.JSON);

tc.highLevelClient().indices().create(request, RequestOptions.DEFAULT);

BulkRequest bulk = new BulkRequest();
bulk.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
{
"properties" : {
"command_line" : {
"type" : "keyword"
},
"event" : {
"properties" : {
"category" : {
"type" : "keyword"
}
}
},
"md5" : {
"type" : "keyword"
},
"parent_process_name": {
"type" : "keyword"
},
"parent_process_path": {
"type" : "keyword"
},
"pid" : {
"type" : "long"
},
"ppid" : {
"type" : "long"
},
"process_name": {
"type" : "keyword"
},
"process_path": {
"type" : "keyword"
},
"subtype" : {
"type" : "keyword"
},
"@timestamp" : {
"type" : "date"
},
"user" : {
"type" : "keyword"
},
"user_name" : {
"type" : "keyword"
},
"user_domain": {
"type" : "keyword"
},
"hostname" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
},
"opcode" : {
"type" : "long"
},
"file_name" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
},
"serial_event_id" : {
"type" : "long"
},
"source_address" : {
"type" : "ip"
},
"exit_code" : {
"type" : "long"
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1126,30 +1126,6 @@ query = '''
file where between(file_path, "dev", ".json", true) == "\\testlogs\\something"
'''

[[queries]]
expected_event_ids = [75304, 75305]
query = '''
network where cidrMatch(source_address, "10.6.48.157/8")
'''

[[queries]]
expected_event_ids = []
query = '''
network where cidrMatch(source_address, "192.168.0.0/16")
'''

[[queries]]
expected_event_ids = [75304, 75305]
query = '''
network where cidrMatch(source_address, "192.168.0.0/16", "10.6.48.157/8")

'''
[[queries]]
expected_event_ids = [75304, 75305]
query = '''
network where cidrMatch(source_address, "0.0.0.0/0")
'''

[[queries]]
expected_event_ids = [7, 14, 22, 29, 44]
query = '''
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

package org.elasticsearch.xpack.eql.expression.function;

import org.elasticsearch.xpack.eql.expression.function.scalar.string.CIDRMatch;
import org.elasticsearch.xpack.eql.expression.function.scalar.string.Substring;
import org.elasticsearch.xpack.ql.expression.function.FunctionDefinition;
import org.elasticsearch.xpack.ql.expression.function.FunctionRegistry;
Expand All @@ -24,6 +25,7 @@ private static FunctionDefinition[][] functions() {
// String
new FunctionDefinition[] {
def(Substring.class, Substring::new, "substring"),
def(CIDRMatch.class, CIDRMatch::new, "cidrmatch"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer listing the functions in alphabetical order.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. Updated.

},
};
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

package org.elasticsearch.xpack.eql.expression.function.scalar.string;

import org.elasticsearch.xpack.ql.expression.Expression;
import org.elasticsearch.xpack.ql.expression.Expressions;
import org.elasticsearch.xpack.ql.expression.Expressions.ParamOrdinal;
import org.elasticsearch.xpack.ql.expression.function.scalar.ScalarFunction;
import org.elasticsearch.xpack.ql.expression.gen.script.ScriptTemplate;
import org.elasticsearch.xpack.ql.expression.predicate.logical.Or;
import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.Equals;
import org.elasticsearch.xpack.ql.tree.NodeInfo;
import org.elasticsearch.xpack.ql.tree.Source;
import org.elasticsearch.xpack.ql.type.DataType;
import org.elasticsearch.xpack.ql.type.DataTypes;
import org.elasticsearch.xpack.ql.util.CollectionUtils;

import java.util.List;

import static java.util.Collections.singletonList;
import static org.elasticsearch.xpack.ql.expression.TypeResolutions.isIPAndExact;
import static org.elasticsearch.xpack.ql.expression.TypeResolutions.isStringAndExact;

/**
* EQL specific cidrMatch function
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personal preference, but I would like to see a useful description of the function. Not even the original EQL documentation is clear imo: https://eql.readthedocs.io/en/latest/query-guide/functions.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a couple of more lines with a link to the EQL doc.

*/
public class CIDRMatch extends ScalarFunction {

private final Expression field;
private final List<Expression> addresses;

public CIDRMatch(Source source, Expression field, List<Expression> addresses) {
super(source, CollectionUtils.combine(singletonList(field), addresses));
this.field = field;
this.addresses = addresses;
}

@Override
protected TypeResolution resolveType() {
if (!childrenResolved()) {
return new TypeResolution("Unresolved children");
}

TypeResolution resolution = isIPAndExact(field, sourceText(), Expressions.ParamOrdinal.FIRST);
if (resolution.unresolved()) {
return resolution;
}

for (Expression addr: addresses) {
// Currently we have limited enum for ordinal numbers
// So just using default here for error messaging
resolution = isStringAndExact(addr, sourceText(), ParamOrdinal.DEFAULT);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current enum for error messages doesn't work great for variadic type of function. Open to suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a method here in my wildcard PR:

public static ParamOrdinal fromIndex(int index) {
switch (index) {
case 0: return ParamOrdinal.FIRST;
case 1: return ParamOrdinal.SECOND;
case 2: return ParamOrdinal.THIRD;
case 3: return ParamOrdinal.FOURTH;
default: return ParamOrdinal.DEFAULT;
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, should these fields also be isStringAndExact? and an isFoldable check?
Here's how I approached that

lastResolution = isFoldable(p, sourceText(), ParamOrdinal.fromIndex(index));
if (lastResolution.unresolved()) {
break;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! Will update after your PR merges.
The ParamOrdinal falls back to ParamOrdinal.DEFAULT after 4 params anyways. Maybe make enum longer if we still want to use enum there?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For how to handle functions that deal with a variable list of parameters, I suggest having a look at org.elasticsearch.xpack.sql.expression.predicate.conditional.Case or org.elasticsearch.xpack.sql.expression.predicate.conditional.Coalesce since these are functions that have a variable list of parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Relying on the same def builder here that was merged with wildcard function for now.
Updated to align with wildcard impl usage of ParamOrdinal.

if (resolution.unresolved()) {
return resolution;
}
}

return resolution;
}

@Override
public boolean foldable() {
return super.foldable() && field.foldable() && asFunction().foldable();
}

@Override
public Object fold() {
return asFunction().fold();
}

@Override
protected NodeInfo<? extends Expression> info() {
return NodeInfo.create(this, CIDRMatch::new, field, addresses);
}

@Override
public ScriptTemplate asScript() {
throw new UnsupportedOperationException();
}

@Override
public DataType dataType() {
return DataTypes.BOOLEAN;
}

@Override
public Expression replaceChildren(List<Expression> newChildren) {
if (newChildren.size() < 2) {
throw new IllegalArgumentException("expected at least [2] children but received [" + newChildren.size() + "]");
}
return new CIDRMatch(source(), newChildren.get(0), newChildren.subList(1, newChildren.size()));
}

public ScalarFunction asFunction() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update to take into account #54795 - reduces the class and removes a separate optimizer rule.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, will do once #54795 is merged.

ScalarFunction func = null;

for (Expression address: addresses) {
final Equals eq = new Equals(source(), field, address);
func = (func == null) ? eq : new Or(source(), func, eq);
}

return func;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

package org.elasticsearch.xpack.eql.optimizer;

import org.elasticsearch.xpack.eql.expression.function.scalar.string.CIDRMatch;
import org.elasticsearch.xpack.ql.expression.Expression;
import org.elasticsearch.xpack.ql.expression.predicate.logical.Not;
import org.elasticsearch.xpack.ql.expression.predicate.nulls.IsNotNull;
Expand Down Expand Up @@ -48,6 +49,7 @@ protected Iterable<RuleExecutor<LogicalPlan>.Batch> batches() {
new ReplaceNullChecks(),
new PropagateEquals(),
new CombineBinaryComparisons(),
new ReplaceCIDRMatchFunction(),
// prune/elimination
new PruneFilters(),
new PruneLiteralsInOrderBy()
Expand Down Expand Up @@ -129,4 +131,12 @@ protected LogicalPlan rule(Filter filter) {
});
}
}

private static class ReplaceCIDRMatchFunction extends OptimizerRule<Filter> {

@Override
protected LogicalPlan rule(Filter filter) {
return filter.transformExpressionsUp(e -> e instanceof CIDRMatch ? ((CIDRMatch) e).asFunction() : e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,6 @@ public void testFunctionVerificationUnknown() {
error("process where concat(serial_event_id, ':', process_name, opcode) == '5:winINIT.exe3'"));
assertEquals("1:15: Unknown function [between]",
error("process where between(process_name, \"s\", \"e\") == \"yst\""));
assertEquals("1:15: Unknown function [cidrMatch]",
error("network where cidrMatch(source_address, \"192.168.0.0/16\", \"10.6.48.157/8\")"));
assertEquals("1:22: Unknown function [between]",
error("process where length(between(process_name, 'g', 'e')) > 0"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

package org.elasticsearch.xpack.eql.planner;

import org.elasticsearch.xpack.eql.analysis.VerificationException;
import org.elasticsearch.xpack.ql.ParsingException;
import org.elasticsearch.xpack.ql.QlIllegalArgumentException;

public class QueryFolderFailTests extends AbstractQueryFolderTestCase {
Expand All @@ -22,4 +24,34 @@ public void testPropertyEquationInClauseFilterUnsupported() {
String msg = e.getMessage();
assertEquals("Line 1:52: Comparisons against variables are not (currently) supported; offender [parent_process_name] in [==]", msg);
}

public void testCIDRMatchNonIPField() {
VerificationException e = expectThrows(VerificationException.class,
() -> plan("process where cidrMatch(hostname, \"10.0.0.0/8\")"));
String msg = e.getMessage();
assertEquals("Found 1 problem\n" +
"line 1:15: first argument of [cidrMatch(hostname, \"10.0.0.0/8\")] must be [ip], found value [hostname] type [text]", msg);
}

public void testCIDRMatchMissingValue() {
ParsingException e = expectThrows(ParsingException.class,
() -> plan("process where cidrMatch(source_address)"));
String msg = e.getMessage();
assertEquals("line 1:16: error building [cidrmatch]: expects at least two arguments", msg);
}

public void testCIDRMatchAgainstField() {
QlIllegalArgumentException e = expectThrows(QlIllegalArgumentException.class,
() -> plan("process where cidrMatch(source_address, hostname)"));
String msg = e.getMessage();
assertEquals("Line 1:41: Comparisons against variables are not (currently) supported; offender [hostname] in [==]", msg);
}

public void testCIDRMatchNonString() {
VerificationException e = expectThrows(VerificationException.class,
() -> plan("process where cidrMatch(source_address, 12345)"));
String msg = e.getMessage();
assertEquals("Found 1 problem\n" +
"line 1:15: argument of [cidrMatch(source_address, 12345)] must be [string], found value [12345] type [integer]", msg);
}
}
17 changes: 17 additions & 0 deletions x-pack/plugin/eql/src/test/resources/queryfolder_tests.txt
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,20 @@ process where substring(file_name, -4) == '.exe'
InternalEqlScriptUtils.substring(InternalQlScriptUtils.docValue(doc,params.v0),params.v1,params.v2),params.v3))",
"params":{"v0":"file_name.keyword","v1":-4,"v2":null,"v3":".exe"}


cidrMatchFunctionOne
process where cidrMatch(source_address, "10.0.0.0/8")
"term":{"source_address":{"value":"10.0.0.0/8"


cidrMatchFunctionTwo
process where cidrMatch(source_address, "10.0.0.0/8", "192.168.0.0/16")
"term":{"source_address":{"value":"10.0.0.0/8"
"term":{"source_address":{"value":"192.168.0.0/16"


cidrMatchFunctionThree
process where cidrMatch(source_address, "10.0.0.0/8", "192.168.0.0/16", "2001:db8::/32")
"term":{"source_address":{"value":"10.0.0.0/8"
"term":{"source_address":{"value":"192.168.0.0/16"
"term":{"source_address":{"value":"2001:db8::/32"
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
import static org.elasticsearch.xpack.ql.expression.Expressions.name;
import static org.elasticsearch.xpack.ql.type.DataTypes.BOOLEAN;
import static org.elasticsearch.xpack.ql.type.DataTypes.IP;
import static org.elasticsearch.xpack.ql.type.DataTypes.NULL;

public final class TypeResolutions {
Expand All @@ -40,6 +41,10 @@ public static TypeResolution isString(Expression e, String operationName, ParamO
return isType(e, DataTypes::isString, operationName, paramOrd, "string");
}

public static TypeResolution isIP(Expression e, String operationName, ParamOrdinal paramOrd) {
return isType(e, dt -> dt == IP, operationName, paramOrd, "ip");
}

public static TypeResolution isExact(Expression e, String message) {
if (e instanceof FieldAttribute) {
EsField.Exact exact = ((FieldAttribute) e).getExactInfo();
Expand Down Expand Up @@ -73,6 +78,15 @@ public static TypeResolution isStringAndExact(Expression e, String operationName
return isExact(e, operationName, paramOrd);
}

public static TypeResolution isIPAndExact(Expression e, String operationName, ParamOrdinal paramOrd) {
TypeResolution resolution = isIP(e, operationName, paramOrd);
if (resolution.unresolved()) {
return resolution;
}

return isExact(e, operationName, paramOrd);
}

public static TypeResolution isFoldable(Expression e, String operationName, ParamOrdinal paramOrd) {
if (!e.foldable()) {
return new TypeResolution(format(null, "{}argument of [{}] must be a constant, received [{}]",
Expand Down
Loading