[Security Solution][Detections] Adds sequence callout in the exceptions modals for eql rule types #79007

dplumlee · 2020-09-30T18:41:08Z

Summary

Adds a callout warning to inform users of exception-related side effects should they choose to create/edit an exception tied to a eql rule type that has a sequence query.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

dplumlee · 2020-09-30T18:45:04Z

.../plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx

@@ -316,6 +317,13 @@ export const AddExceptionModal = memo(function AddExceptionModal({
    [fetchOrCreateListError, exceptionItemsToAdd]
  );

+  const isRuleEQLSequenceStatement = useMemo((): boolean => {
+    if (maybeRule != null && maybeRule.query != null) {
+      return maybeRule.type === 'eql' && maybeRule.query.startsWith('sequence');


This is the logic I chose to go with for delineating sequence queries and non-sequence queries. When rylands validation pr gets merged, this should cover all cases given that sequence is a saved term in eql, but any comments/suggestions from those more versed in EQL would be appreciated

@rw-access may have ideas about the best way to do this

@dplumlee FYI we have some helpers available to avoid these kinds of hardcoded strings. Since this logic is duplicated in a few spots, I'd say that moving this function out to a shared location makes sense.

nice. for the logic in general, first word is sequence and the second word is not where.
then you'll be set (assuming it's valid syntax in the first place)

is sequence where ... valid eql? I tested using sequence as an event category in the EQL ES api and got an exception, wasn't sure if that just isn't allowed since sequence is a reserved word.

I haven't tried sequence where true in ES EQL, wasn't sure how ANTLR handled it.

elasticmachine · 2020-09-30T18:45:25Z

Pinging @elastic/siem (Team:SIEM)

dplumlee · 2020-09-30T23:24:56Z

x-pack/plugins/security_solution/common/detection_engine/utils.ts

+export const hasEqlSequenceQuery = (ruleQuery: string | undefined): boolean => {
+  if (ruleQuery != null) {
+    const parsedQuery = ruleQuery.split(' ');
+    return parsedQuery[0] === 'sequence' && parsedQuery[1] !== 'where';


@rylnd @rw-access will something like this work, or is it valid syntax to begin an eql query with a leading space?

It is valid, or at least valid to pass to the EQL api, so I just trimmed it that should solve any potential edge cases problems

might wanna add a test case for other forms of whitespace and multiple between sequence and where, but I think this approach is solid.

ah yeah good call. new fix should cover that too, thanks

dplumlee · 2020-10-01T21:49:43Z

@elasticmachine merge upstream

peluja1012 · 2020-10-01T23:50:59Z

...ns/security_solution/public/common/components/exceptions/add_exception_modal/translations.ts

+  'xpack.securitySolution.exceptions.addException.sequenceWarning',
+  {
+    defaultMessage:
+      'This rule is a sequence statement. The exception created will apply to all events in the sequence.',


@paulewing Do you have any preferences on the language here? I'm thinking that the first sentence could be more specific and say something like this:

This rule's query contains an EQL sequence statement.

rw-access · 2020-10-02T07:38:29Z

x-pack/plugins/security_solution/common/detection_engine/utils.test.ts

+    });
+  });
+
+  describe('when a sequence query is passed with extra white space', () => {


thanks for the updates.
how does this work for "sequence\n..."? or "sequence where ..." which should be invalid syntax on the ES side. but if it is accepted syntax, then it's definitely not a sequence query

added coverage for \n but it should work for the sequence where ... case already. Added another test for coverage sake

rw-access · 2020-10-02T17:32:01Z

x-pack/plugins/security_solution/common/detection_engine/utils.ts

@@ -17,6 +17,14 @@ export const hasNestedEntry = (entries: EntriesArray): boolean => {
  return found.length > 0;
 };

+export const hasEqlSequenceQuery = (ruleQuery: string | undefined): boolean => {
+  if (ruleQuery != null) {
+    const parsedQuery = ruleQuery.split(' ').filter((word) => word !== '' && word !== '\n');


It still seems brittle for some reason, since there are four whitespace characters for EQL. sequence\n[x] may slip through.

ruleQuery.trim().split(/[ \t\r\n]+/); should do it the same way ANTLR does

ah cool, i'll switch to that then

dplumlee · 2020-10-02T22:15:44Z

@elasticmachine merge upstream

dplumlee · 2020-10-05T14:42:09Z

@elasticmachine merge upstream

kibanamachine · 2020-10-05T16:25:17Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: bc046d4

Metrics [docs]

async chunks size

id	before	after	diff
`securitySolution`	10.3MB	10.3MB	+3.4KB

History

💔 Build #79651 failed 1defbb6a8a00c9f40c082994cddd55caf2b10263
💛 Build #79356 was flaky 21235d2a55f9344ba9b09a2e4c1e6702b7ae5335
💔 Build #79333 failed 618cd749465ce402bb83fee2e86b23338921b4a6
💚 Build #79293 succeeded 98bcfa403d666417563de7b4428d7b3fbb667f82
💚 Build #79263 succeeded ba79b3542476f834b6bc3830f95880600d3b0a2f

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

FrankHassanabad

LGTM (caveat) I'm not one of the original commenters but it looks like everything was addressed. And if not, well, follow ups are always possible! :-)

…ns modals for eql rule types (#79007) (#79562)

elasticmachine · 2021-09-23T14:24:21Z

Pinging @elastic/security-solution (Team: SecuritySolution)

dplumlee added Team:SIEM v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.10.0 Feature:Detection Rules Security Solution rules and Detection Engine labels Sep 30, 2020

dplumlee self-assigned this Sep 30, 2020

dplumlee commented Sep 30, 2020

View reviewed changes

dplumlee marked this pull request as ready for review September 30, 2020 18:45

dplumlee requested review from a team as code owners September 30, 2020 18:45

dplumlee commented Sep 30, 2020

View reviewed changes

dplumlee force-pushed the modal-sequence-callout branch from dde40d8 to abd131a Compare October 1, 2020 22:36

peluja1012 reviewed Oct 1, 2020

View reviewed changes

rw-access reviewed Oct 2, 2020

View reviewed changes

dplumlee force-pushed the modal-sequence-callout branch 2 times, most recently from 98bcfa4 to 618cd74 Compare October 2, 2020 20:48

dplumlee added 7 commits October 5, 2020 08:54

adds sequence callout for eql rule types

01a56a1

moves logic to utils file

2b62c7c

trims query string before logic

78b4c59

fixes logic and adds tests

5f29631

adds coverage for new line

e489d8b

adds regex for validation

5db3919

changes language

bc046d4

dplumlee force-pushed the modal-sequence-callout branch from 1defbb6 to bc046d4 Compare October 5, 2020 14:54

FrankHassanabad approved these changes Oct 5, 2020

View reviewed changes

dplumlee merged commit 6a173ba into elastic:master Oct 5, 2020

dplumlee deleted the modal-sequence-callout branch October 5, 2020 20:39

dplumlee mentioned this pull request Oct 5, 2020

[7.x] [Security Solution][Detections] Adds sequence callout in the exceptions modals for eql rule types (#79007) #79562

Merged

dplumlee added a commit that referenced this pull request Oct 6, 2020

[Security Solution][Detections] Adds sequence callout in the exceptio…

ceb8cdd

…ns modals for eql rule types (#79007) (#79562)

MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Sep 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Detections] Adds sequence callout in the exceptions modals for eql rule types #79007

[Security Solution][Detections] Adds sequence callout in the exceptions modals for eql rule types #79007

dplumlee commented Sep 30, 2020 •

edited

Loading

dplumlee Sep 30, 2020

marshallmain Sep 30, 2020

rylnd Sep 30, 2020

rw-access Sep 30, 2020

marshallmain Sep 30, 2020

rw-access Sep 30, 2020

elasticmachine commented Sep 30, 2020

dplumlee Sep 30, 2020 •

edited

Loading

dplumlee Oct 1, 2020

rw-access Oct 1, 2020 •

edited

Loading

dplumlee Oct 2, 2020

dplumlee commented Oct 1, 2020

peluja1012 Oct 1, 2020

rw-access Oct 2, 2020

dplumlee Oct 2, 2020

rw-access Oct 2, 2020

dplumlee Oct 2, 2020

dplumlee commented Oct 2, 2020

dplumlee commented Oct 5, 2020

kibanamachine commented Oct 5, 2020

FrankHassanabad left a comment

elasticmachine commented Sep 23, 2021

[Security Solution][Detections] Adds sequence callout in the exceptions modals for eql rule types #79007

[Security Solution][Detections] Adds sequence callout in the exceptions modals for eql rule types #79007

Conversation

dplumlee commented Sep 30, 2020 • edited Loading

Summary

Checklist

For maintainers

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Sep 30, 2020

dplumlee Sep 30, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rw-access Oct 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dplumlee commented Oct 1, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dplumlee commented Oct 2, 2020

dplumlee commented Oct 5, 2020

kibanamachine commented Oct 5, 2020

💚 Build Succeeded

Metrics [docs]

async chunks size

History

FrankHassanabad left a comment

Choose a reason for hiding this comment

elasticmachine commented Sep 23, 2021

dplumlee commented Sep 30, 2020 •

edited

Loading

dplumlee Sep 30, 2020 •

edited

Loading

rw-access Oct 1, 2020 •

edited

Loading