Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
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 @@ -1088,30 +1088,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.EndsWith;
import org.elasticsearch.xpack.eql.expression.function.scalar.string.Length;
import org.elasticsearch.xpack.eql.expression.function.scalar.string.StartsWith;
Expand All @@ -27,6 +28,7 @@ private static FunctionDefinition[][] functions() {
// Scalar functions
// String
new FunctionDefinition[] {
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.

def(EndsWith.class, EndsWith::new, "endswith"),
def(Length.class, Length::new, "length"),
def(StartsWith.class, StartsWith::new, "startswith"),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
/*
* 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.isFoldable;
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.

* Returns true if the source address matches any of the provided CIDR blocks.
* Refer to: https://eql.readthedocs.io/en/latest/query-guide/functions.html#cidrMatch
*/
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;
}
}

int index = 1;

for (Expression addr: addresses) {

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

resolution = isStringAndExact(addr, sourceText(), ParamOrdinal.fromIndex(index));
if (resolution.unresolved()) {
break;
}

index++;
}

return resolution;
}

@Override
public boolean foldable() {
return 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.eql.expression.function.scalar.string.Wildcard;
import org.elasticsearch.xpack.eql.util.StringUtils;
import org.elasticsearch.xpack.ql.expression.Expression;
Expand Down Expand Up @@ -49,6 +50,7 @@ protected Iterable<RuleExecutor<LogicalPlan>.Batch> batches() {
new ReplaceNullChecks(),
new PropagateEquals(),
new CombineBinaryComparisons(),
new ReplaceCIDRMatchFunction(),
new ReplaceWildcardFunction(),
// prune/elimination
new PruneFilters(),
Expand Down Expand Up @@ -127,4 +129,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 @@ -149,8 +149,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 @@ -80,4 +80,35 @@ public void testWildcardWithNumericField() {
assertEquals("Found 1 problem\n" +
"line 1:15: first argument of [wildcard(pid, '*.exe')] must be [string], found value [pid] type [long]", 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() {
VerificationException e = expectThrows(VerificationException.class,
() -> plan("process where cidrMatch(source_address, hostname)"));
String msg = e.getMessage();
assertEquals("Found 1 problem\n" +
"line 1:15: second argument of [cidrMatch(source_address, hostname)] must be a constant, received [hostname]", 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);
}
}
19 changes: 18 additions & 1 deletion x-pack/plugin/eql/src/test/resources/queryfolder_tests.txt
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,24 @@ InternalEqlScriptUtils.substring(InternalQlScriptUtils.docValue(doc,params.v0),p
"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"


wildcardFunctionSingleArgument
process where wildcard(process_path, "*\\red_ttp\\wininit.*")
"wildcard":{"process_path":{"wildcard":"*\\\\red_ttp\\\\wininit.*"
Expand All @@ -119,4 +137,3 @@ process where wildcard(process_path, "*\\red_ttp\\wininit.*", "*\\abc\\*", "*def
"wildcard":{"process_path":{"wildcard":"*\\\\red_ttp\\\\wininit.*"
"wildcard":{"process_path":{"wildcard":"*\\\\abc\\\\*"
"wildcard":{"process_path":{"wildcard":"*def*"

Loading