Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add an "exists" check for "not" condition in sigma rules #852

Merged
merged 16 commits into from
Mar 8, 2024
Merged

Add an "exists" check for "not" condition in sigma rules #852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
872b14f
test design
jowg-amazon Feb 8, 2024
2ac748b
working version
jowg-amazon Feb 8, 2024
d0831f2
cleaning up
jowg-amazon Feb 8, 2024
27333ec
testing
jowg-amazon Feb 9, 2024
1ba0117
working version
jowg-amazon Feb 10, 2024
20086b2
working version
jowg-amazon Feb 10, 2024
9a01b04
refactored querybackend
jowg-amazon Feb 10, 2024
034097b
working on tests
jowg-amazon Feb 12, 2024
0769096
fixed alerting and finding tests
jowg-amazon Feb 12, 2024
5ddaab6
fix correlation tests
jowg-amazon Feb 12, 2024
444472d
working all tests
jowg-amazon Feb 12, 2024
fa2b9ae
moved test and changed alias for adldap
jowg-amazon Feb 12, 2024
ec8d4c5
added more tests
jowg-amazon Feb 12, 2024
5efdc79
cleanup code
jowg-amazon Feb 12, 2024
e774d02
remove exists flag
jowg-amazon Feb 29, 2024
c71312a
Merge branch 'main' into add_exists_check
jowg-amazon Mar 7, 2024
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
142 changes: 105 additions & 37 deletions src/main/java/org/opensearch/securityanalytics/rules/backend/OSQueryBackend.java
View file
Open in desktop

Large diffs are not rendered by default.

102 changes: 56 additions & 46 deletions src/main/java/org/opensearch/securityanalytics/rules/backend/QueryBackend.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
*/
package org.opensearch.securityanalytics.rules.backend;

import org.opensearch.commons.alerting.aggregation.bucketselectorext.BucketSelectorExtAggregationBuilder;
import org.opensearch.search.aggregations.AggregationBuilder;
import org.opensearch.securityanalytics.rules.aggregation.AggregationItem;
import org.opensearch.securityanalytics.rules.backend.OSQueryBackend.AggregationQueries;
import org.opensearch.securityanalytics.rules.condition.ConditionAND;
Expand Down Expand Up @@ -47,7 +45,6 @@
import java.util.Set;

public abstract class QueryBackend {

private boolean convertOrAsIn;
private boolean convertAndAsIn;
private boolean collectErrors;
Expand Down Expand Up @@ -85,15 +82,15 @@

Object query;
if (conditionItem instanceof ConditionAND) {
query = this.convertCondition(new ConditionType(Either.left(AnyOneOf.leftVal((ConditionAND) conditionItem))));
query = this.convertCondition(new ConditionType(Either.left(AnyOneOf.leftVal((ConditionAND) conditionItem))), false, false);
Copy link
Member

Choose a reason for hiding this comment

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

why is the applyDeMorgans always false? do we even need it??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

applyDeMorgans starts off as false but we need it since we are recursively traversing the tree. It gets set to true (ie. we need to apply deMorgans) only if there is an AND or OR condition when it is an ancestor of a NOT.

Ex.

  1. NOT A OR B -> do not apply deMorgans
  2. NOT(A OR B) -> need to apply deMorgans
  • This is because when we append the AND exists:field to the query, if we don't do deMorgans first but directly apply it directly to the query, an incorrect query would be constructed.

Ex.
Incorrect: NOT(A OR B) -> NOT(A AND exists:A OR B AND exists:B) -> NOT A OR exists:A AND NOT B OR exists:B
Correct: NOT(A OR B) -> NOT A AND NOT B -> NOT A AND exists:A AND NOT B AND exists:B

} else if (conditionItem instanceof ConditionOR) {
query = this.convertCondition(new ConditionType(Either.left(AnyOneOf.middleVal((ConditionOR) conditionItem))));
query = this.convertCondition(new ConditionType(Either.left(AnyOneOf.middleVal((ConditionOR) conditionItem))), false, false);
} else if (conditionItem instanceof ConditionNOT) {
query = this.convertCondition(new ConditionType(Either.left(AnyOneOf.rightVal((ConditionNOT) conditionItem))));
query = this.convertCondition(new ConditionType(Either.left(AnyOneOf.rightVal((ConditionNOT) conditionItem))), true, false);
} else if (conditionItem instanceof ConditionFieldEqualsValueExpression) {
query = this.convertCondition(new ConditionType(Either.right(Either.left((ConditionFieldEqualsValueExpression) conditionItem))));
query = this.convertCondition(new ConditionType(Either.right(Either.left((ConditionFieldEqualsValueExpression) conditionItem))), false, false);
} else {
query = this.convertCondition(new ConditionType(Either.right(Either.right((ConditionValueExpression) conditionItem))));
query = this.convertCondition(new ConditionType(Either.right(Either.right((ConditionValueExpression) conditionItem))), false, false);
}
queries.add(query);
if (aggItem != null) {
Expand All @@ -113,30 +110,41 @@
return queries;
}

public Object convertCondition(ConditionType conditionType) throws SigmaValueError {
public Object convertCondition(ConditionType conditionType, boolean isConditionNot, boolean applyDeMorgans) throws SigmaValueError {
Copy link
Member

Choose a reason for hiding this comment

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

are both these newly added bool params even necessary?
can't we just set them as necessary for convertConditionNot

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

see above

if (conditionType.isConditionOR()) {
if (this.decideConvertConditionAsInExpression(Either.right(conditionType.getConditionOR()))) {
return this.convertConditionAsInExpression(Either.right(conditionType.getConditionOR()));
return this.convertConditionAsInExpression(Either.right(conditionType.getConditionOR()), isConditionNot, applyDeMorgans );

Check warning on line 116 in src/main/java/org/opensearch/securityanalytics/rules/backend/QueryBackend.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/securityanalytics/rules/backend/QueryBackend.java#L116 

Added line #L116 was not covered by tests
} else {
return this.convertConditionOr(conditionType.getConditionOR());
return this.convertConditionOr(conditionType.getConditionOR(), isConditionNot, applyDeMorgans);
}
} else if (conditionType.isConditionAND()) {
if (this.decideConvertConditionAsInExpression(Either.left(conditionType.getConditionAND()))) {
return this.convertConditionAsInExpression(Either.left(conditionType.getConditionAND()));
return this.convertConditionAsInExpression(Either.left(conditionType.getConditionAND()), isConditionNot, applyDeMorgans);

Check warning on line 122 in src/main/java/org/opensearch/securityanalytics/rules/backend/QueryBackend.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/securityanalytics/rules/backend/QueryBackend.java#L122 

Added line #L122 was not covered by tests
} else {
return this.convertConditionAnd(conditionType.getConditionAND());
return this.convertConditionAnd(conditionType.getConditionAND(), isConditionNot, applyDeMorgans);
}
} else if (conditionType.isConditionNOT()) {
return this.convertConditionNot(conditionType.getConditionNOT());
return this.convertConditionNot(conditionType.getConditionNOT(), isConditionNot, applyDeMorgans);
} else if (conditionType.isEqualsValueExpression()) {
return this.convertConditionFieldEqVal(conditionType.getEqualsValueExpression());
// check to see if conditionNot is an ancestor of the parse tree, otherwise return as normal
if (isConditionNot) {
return this.convertConditionFieldEqValNot(conditionType, isConditionNot, applyDeMorgans);
} else {
return this.convertConditionFieldEqVal(conditionType.getEqualsValueExpression(), isConditionNot, applyDeMorgans);
}
} else if (conditionType.isValueExpression()) {
return this.convertConditionVal(conditionType.getValueExpression());
return this.convertConditionVal(conditionType.getValueExpression(), applyDeMorgans);
} else {
throw new IllegalArgumentException("Unexpected data type in condition parse tree");
}
}

public String convertConditionFieldEqValNot(ConditionType conditionType, boolean isConditionNot, boolean applyDeMorgans) throws SigmaValueError {
String baseString = this.convertConditionFieldEqVal(conditionType.getEqualsValueExpression(), isConditionNot, applyDeMorgans).toString();
String addExists = this.convertExistsField(conditionType.getEqualsValueExpression()).toString();
return String.format(Locale.getDefault(), ("%s" + "%s"), baseString, addExists);
}

public boolean decideConvertConditionAsInExpression(Either<ConditionAND, ConditionOR> condition) {
if ((!this.convertOrAsIn && condition.isRight()) || (!this.convertAndAsIn && condition.isLeft())) {
return false;
Expand Down Expand Up @@ -181,74 +189,76 @@
}
}

public abstract Object convertConditionAsInExpression(Either<ConditionAND, ConditionOR> condition);
public abstract Object convertConditionAsInExpression(Either<ConditionAND, ConditionOR> condition, boolean isConditionNot, boolean applyDeMorgans);

public abstract Object convertConditionAnd(ConditionAND condition);
public abstract Object convertConditionAnd(ConditionAND condition, boolean isConditionNot, boolean applyDeMorgans);

public abstract Object convertConditionOr(ConditionOR condition);
public abstract Object convertConditionOr(ConditionOR condition, boolean isConditionNot, boolean applyDeMorgans);

public abstract Object convertConditionNot(ConditionNOT condition);
public abstract Object convertConditionNot(ConditionNOT condition, boolean isConditionNot, boolean applyDeMorgans);

public Object convertConditionFieldEqVal(ConditionFieldEqualsValueExpression condition) throws SigmaValueError {
public Object convertConditionFieldEqVal(ConditionFieldEqualsValueExpression condition, boolean isConditionNot, boolean applyDeMorgans) throws SigmaValueError {
if (condition.getValue() instanceof SigmaString) {
return this.convertConditionFieldEqValStr(condition);
return this.convertConditionFieldEqValStr(condition, applyDeMorgans);
} else if (condition.getValue() instanceof SigmaNumber) {
return this.convertConditionFieldEqValNum(condition);
return this.convertConditionFieldEqValNum(condition, applyDeMorgans);
} else if (condition.getValue() instanceof SigmaBool) {
return this.convertConditionFieldEqValBool(condition);
return this.convertConditionFieldEqValBool(condition, applyDeMorgans);
} else if (condition.getValue() instanceof SigmaRegularExpression) {
return this.convertConditionFieldEqValRe(condition);
return this.convertConditionFieldEqValRe(condition, applyDeMorgans);
} else if (condition.getValue() instanceof SigmaCIDRExpression) {
return this.convertConditionFieldEqValCidr(condition);
return this.convertConditionFieldEqValCidr(condition, applyDeMorgans);
} else if (condition.getValue() instanceof SigmaCompareExpression) {
return this.convertConditionFieldEqValOpVal(condition);
return this.convertConditionFieldEqValOpVal(condition, applyDeMorgans);
} else if (condition.getValue() instanceof SigmaNull) {
return this.convertConditionFieldEqValNull(condition);
return this.convertConditionFieldEqValNull(condition, applyDeMorgans);
}/* TODO: below methods will be supported when Sigma Expand Modifier is supported.
else if (condition.getValue() instanceof SigmaQueryExpression) {
return this.convertConditionFieldEqValQueryExpr(condition);
}*/ else if (condition.getValue() instanceof SigmaExpansion) {
return this.convertConditionFieldEqValQueryExpansion(condition);
return this.convertConditionFieldEqValQueryExpansion(condition, isConditionNot, applyDeMorgans);
} else {
throw new IllegalArgumentException("Unexpected value type class in condition parse tree: " + condition.getValue().getClass().getName());
}
}

public abstract Object convertConditionFieldEqValStr(ConditionFieldEqualsValueExpression condition) throws SigmaValueError;
public abstract Object convertConditionFieldEqValStr(ConditionFieldEqualsValueExpression condition, boolean applyDeMorgans) throws SigmaValueError;

public abstract Object convertConditionFieldEqValNum(ConditionFieldEqualsValueExpression condition, boolean applyDeMorgans);

public abstract Object convertConditionFieldEqValNum(ConditionFieldEqualsValueExpression condition);
public abstract Object convertConditionFieldEqValBool(ConditionFieldEqualsValueExpression condition, boolean applyDeMorgans);

public abstract Object convertConditionFieldEqValBool(ConditionFieldEqualsValueExpression condition);
public abstract Object convertConditionFieldEqValRe(ConditionFieldEqualsValueExpression condition, boolean applyDeMorgans);

public abstract Object convertConditionFieldEqValRe(ConditionFieldEqualsValueExpression condition);
public abstract Object convertConditionFieldEqValCidr(ConditionFieldEqualsValueExpression condition, boolean applyDeMorgans);

public abstract Object convertConditionFieldEqValCidr(ConditionFieldEqualsValueExpression condition);
public abstract Object convertConditionFieldEqValOpVal(ConditionFieldEqualsValueExpression condition, boolean applyDeMorgans);

public abstract Object convertConditionFieldEqValOpVal(ConditionFieldEqualsValueExpression condition);
public abstract Object convertConditionFieldEqValNull(ConditionFieldEqualsValueExpression condition, boolean applyDeMorgans);

public abstract Object convertConditionFieldEqValNull(ConditionFieldEqualsValueExpression condition);
public abstract Object convertExistsField(ConditionFieldEqualsValueExpression condition);

/* public abstract Object convertConditionFieldEqValQueryExpr(ConditionFieldEqualsValueExpression condition);*/
/* public abstract Object convertConditionFieldEqValQueryExpr(ConditionFieldEqualsValueExpression condition);*/

public Object convertConditionFieldEqValQueryExpansion(ConditionFieldEqualsValueExpression condition) {
public Object convertConditionFieldEqValQueryExpansion(ConditionFieldEqualsValueExpression condition, boolean isConditionNot, boolean applyDeMorgans) {
List<Either<AnyOneOf<ConditionItem, ConditionFieldEqualsValueExpression, ConditionValueExpression>, String>> args = new ArrayList<>();
for (SigmaType sigmaType: ((SigmaExpansion) condition.getValue()).getValues()) {
args.add(Either.left(AnyOneOf.middleVal(new ConditionFieldEqualsValueExpression(condition.getField(), sigmaType))));
}

ConditionOR conditionOR = new ConditionOR(false, args);
return this.convertConditionOr(conditionOR);
return this.convertConditionOr(conditionOR, isConditionNot, applyDeMorgans);
}

public Object convertConditionVal(ConditionValueExpression condition) throws SigmaValueError {
public Object convertConditionVal(ConditionValueExpression condition, boolean applyDeMorgans) throws SigmaValueError {
if (condition.getValue() instanceof SigmaString) {
return this.convertConditionValStr(condition);
return this.convertConditionValStr(condition, applyDeMorgans);
} else if (condition.getValue() instanceof SigmaNumber) {
return this.convertConditionValNum(condition);
return this.convertConditionValNum(condition, applyDeMorgans);
} else if (condition.getValue() instanceof SigmaBool) {
throw new SigmaValueError("Boolean values can't appear as standalone value without a field name.");
} else if (condition.getValue() instanceof SigmaRegularExpression) {
return this.convertConditionValRe(condition);
return this.convertConditionValRe(condition, applyDeMorgans);
}/* else if (condition.getValue() instanceof SigmaCIDRExpression) {
throw new SigmaValueError("CIDR values can't appear as standalone value without a field name.");
} else if (condition.getValue() instanceof SigmaQueryExpression) {
Expand All @@ -258,11 +268,11 @@
}
}

public abstract Object convertConditionValStr(ConditionValueExpression condition) throws SigmaValueError;
public abstract Object convertConditionValStr(ConditionValueExpression condition, boolean applyDeMorgans) throws SigmaValueError;

public abstract Object convertConditionValNum(ConditionValueExpression condition);
public abstract Object convertConditionValNum(ConditionValueExpression condition, boolean applyDeMorgans);

public abstract Object convertConditionValRe(ConditionValueExpression condition);
public abstract Object convertConditionValRe(ConditionValueExpression condition, boolean applyDeMorgans);

/* public abstract Object convertConditionValQueryExpr(ConditionValueExpression condition);*/

Expand Down
Loading
Loading