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

[Detection Engine] - EQL timestamp override not taking as expected #158326

Closed
yctercero opened this issue May 23, 2023 · 9 comments
Closed

[Detection Engine] - EQL timestamp override not taking as expected #158326

yctercero opened this issue May 23, 2023 · 9 comments
Assignees
@rylnd @kqualters-elastic
Labels
bug Fixes for quality problems that affect the customer experience consider-next Feature:Event Correlation (EQL) Rule Security Solution Event Correlation (EQL) rule type impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Team:Detection Engine Security Solution Detection Engine Area Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Investigations Security Solution Investigations Team Team:Threat Hunting Security Solution Threat Hunting Team

Comments

@yctercero
Copy link
Contributor

yctercero commented May 23, 2023 •
edited
Loading

Describe the bug:

On both the rule creation flow and in timeline, when a timestamp override field is selected for an EQL rule, it is not honored. In some cases, it results in an EQL validation error, blocking user.

Kibana/Elasticsearch Stack version:
Found in 8.7 but looks like possibly it's a 10 month old bug,

Steps to reproduce:

Create an index that uses timestamp instead of @timestamp.

  1. Go to create rule
  2. Select EQL
  3. Input a valid eql query
  4. Notice validation error about timestamp
  5. Select timestamp override to be timestamp
  6. Notice nothing changes

Current behavior:
Timestamp override is not respected.

Expected behavior:
Timestamp override is respected and no validation error occurs.

Screenshots (if relevant):
image (1)
image (2)

Any additional context (logs, chat logs, magical formulas, etc.):
@kqualters-elastic found one area where the timestamp field is hardcoded to @timestamp. Also need to make sure it's passed through for the query validation
@yctercero yctercero added bug Fixes for quality problems that affect the customer experience impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Feature:Event Correlation (EQL) Rule Security Solution Event Correlation (EQL) rule type Team:Detection Engine Security Solution Detection Engine Area labels May 23, 2023
@elasticmachine
Copy link
Contributor

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

@yctercero yctercero added the Team:Threat Hunting Security Solution Threat Hunting Team label May 23, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

@MadameSheema MadameSheema added the Team:Threat Hunting:Investigations Security Solution Investigations Team label May 24, 2023
@rylnd
Copy link
Contributor

rylnd commented Feb 22, 2024

I looked into this briefly, and there looks to be an EQL-specific field that we use for this purpose: timestampField, and that timestampOverride is effectively unused.

At the product level, I'm not sure if we want to support both of these rule options, and whether they're equivalent in functionality (and precedence). It would certainly be simplest to just remove timestampField and use timestampOverride instead; I think this bug is indicative of the fact that the former is unexpected and differs from the rest of the rule types, unless I'm misunderstanding the semantics of that field.

@marshallmain
Copy link
Contributor

timestampField controls the field that is used for ordering events within a sequence, whereas the timestamp override controls which events are in range of the rule's query. We have a short blurb about it here. The idea is that with late arriving events you may want the rule to always query the events that just arrived, but run sequence logic on the events that depends on the order they actually occurred on the source.

@rylnd
Copy link
Contributor

rylnd commented Feb 23, 2024

Thanks @marshallmain, that's useful context. I wrote some integration tests to document the current behavior/interaction of these two fields, with the following conclusions:

  1. If @timestamp is present, then neither of the fields are required since they both fall back to @timestamp
  2. If @timestamp is absent:
    1. Both fields are required, as neither has an appropriate fallback
    2. Omitting timestamp_field results in an error, and no alerts are generated
    3. Omitting timestamp_override results in a warning (actually, two identical warnings), and no alerts are generated
    4. The UI does not allow you to create a valid EQL rule, as you cannot proceed past the Define step to set the required timestamp_override

Focusing on the UI piece (the timeline hardcoding seems like a related but separate issue): I think the quickest fix here would be to link the two and set timestamp_override to the value of timestamp_field if it's unspecified; that would provide the expected behavior documented here. However, that might be unexpected/undesirable if one does have a @timestamp field to fall back on (and I'm not sure if we can make that determination in the UI, or whether there's precedent for that).

@rylnd rylnd self-assigned this Feb 23, 2024
@marshallmain
Copy link
Contributor

It looks like the timestamp_field is not being passed into the EQL validation request here, as I'm not seeing timestamp_field show up in the actual network request. If we pass it in then the query validation should work and allow the rule to be created even without timestamp_override being set. I think we should avoid linking the 2 fields together if possible so they stay independent.

@rylnd
Copy link
Contributor

rylnd commented Feb 24, 2024

Good call; I realized there might be some preview- or validation-specific behavior contributing to this, but neglected to mention that above.

I'll make these changes and update those added tests 👍

@yctercero
Copy link
Contributor Author

Not sure if it's already the case but this would be a great technical design decision to add to the dev docs as well.

@rylnd rylnd mentioned this issue Mar 12, 2024
Merged
4 tasks
rylnd added a commit that referenced this issue Mar 19, 2024
@rylnd
[Security Solution] Incorporates EQL options in EQL query validation …
5be91c9 
…on both Rule Creation and Timeline (#178468)

## Summary

This PR updates the Detection Rule Creation and Timeline forms to
account for the new(er) EQL options fields (`timestamp_field`,
`event_category_field`, and `tiebreaker_field`) when validating the EQL
query. While the rule query and timeline query (respectively) would
correctly persist and use these options, they were unused during EQL
validation, meaning that in certain situations it was impossible to
produce a "valid" (from the perspective of the form) EQL query if your
data necessitated one of those options (see
#158326 for more details).

### Changes
The above was accomplished by:

1. Adding a hidden form field to the UI for `eqlOptions`
* A lack of a field component meant that `formData` never held any value
for this field
* This meant that the form-based validations (i.e. `eqlValidator`) would
not have access to `eqlOptions`
2. Updating the `onOptionsChange` hook to additionally inform the form
of changes to `eqlOptions`
* This allows the form to have updated values for `eqlOptions` with
minimal changes
* Not ideal from a data-flow perspective, since the field changes are
updating two copies of `eqlOptions`. However, we're only using the "form
copy" for the validation call, and everything else works as it did
before.

 
### Additional changes
__Update__: I've reverted the described changes to the default timeline
values below, and moved that to a [separate
issue](elastic/security-team#8892).

In the course of performing the above tasks for timeline, I found the
behavior there to be slightly different: we were specifying default
values for these fields, which was redundant in some cases
(`timestamp_field`, `event_category_field`), and invalid in the case of
`tiebreaker_field`. You can see here how the `tiebreaker_field` must be
changed to make the query valid:


https://github.com/elastic/kibana/assets/657252/556bc998-c147-4825-8bde-5d8d03873e75


~~I opted to remove these default values, as well as the defaulting
behavior from the search strategy call (see
10a47fd). While these values may be
persisted in existing saved timelines, this makes the EQL workflow much
cleaner for new users, and those users with persisted EQL options can
now modify and validate those options on the fly.~~

#### Steps to Review

For @elastic/kibana-visualizations and/or @elastic/kibana-data-discovery
: I've added a few tests to the EQL search strategy that document
existing behavior RE EQL option fields. Nothing about the search
strategy itself was changed. If you think the tests aren't useful, I'm
happy to delete them.

1. Create an index without a `@timestamp` field, and insert some data:
    <details>
    <summary>Dev Tools</summary>

       PUT timestamp/
       {
         "mappings": {
           "dynamic": "strict",
           "properties": {
             "timestamp": {
               "type": "date",
               "format": "epoch_second"
             },
             "locale": {
               "type": "keyword"
             },
             "created_at": {
               "type": "date",
               "format": "epoch_second"
             },
             "event": {
               "properties": {
                 "category": {
                   "ignore_above": 1024,
                   "type": "keyword"
                 }
               }
             }
           }
         }
       }

       PUT timestamp/_doc/4
       {
         "timestamp": 1710211630,
         "locale": "es",
         "created_at": 1710211630,
         "event.category": "configuration"
       }
    </details>
2. Create a new EQL detection rule, and choose `timestamp` as the index
pattern
3. Type `any where true` as the query, and observe a validation error
(`Found 1 problem line -1:-1: Unknown column [@timestamp]`)
4. Open EQL Settings popover, and select `timestamp` from the `Timestamp
field` dropdown
5. Observe that the query has been revalidated, and found valid.


### New Behavior:


https://github.com/elastic/kibana/assets/657252/22a63363-d597-4dfd-8f9a-647f4084ec0e



### Related Issues
* Addresses #158326

### Checklist

Delete any items that are not applicable to this PR.

- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Mar 19, 2024
@rylnd
[Security Solution] Incorporates EQL options in EQL query validation …
fe51566 
…on both Rule Creation and Timeline (elastic#178468)

## Summary

This PR updates the Detection Rule Creation and Timeline forms to
account for the new(er) EQL options fields (`timestamp_field`,
`event_category_field`, and `tiebreaker_field`) when validating the EQL
query. While the rule query and timeline query (respectively) would
correctly persist and use these options, they were unused during EQL
validation, meaning that in certain situations it was impossible to
produce a "valid" (from the perspective of the form) EQL query if your
data necessitated one of those options (see
elastic#158326 for more details).

### Changes
The above was accomplished by:

1. Adding a hidden form field to the UI for `eqlOptions`
* A lack of a field component meant that `formData` never held any value
for this field
* This meant that the form-based validations (i.e. `eqlValidator`) would
not have access to `eqlOptions`
2. Updating the `onOptionsChange` hook to additionally inform the form
of changes to `eqlOptions`
* This allows the form to have updated values for `eqlOptions` with
minimal changes
* Not ideal from a data-flow perspective, since the field changes are
updating two copies of `eqlOptions`. However, we're only using the "form
copy" for the validation call, and everything else works as it did
before.

### Additional changes
__Update__: I've reverted the described changes to the default timeline
values below, and moved that to a [separate
issue](elastic/security-team#8892).

In the course of performing the above tasks for timeline, I found the
behavior there to be slightly different: we were specifying default
values for these fields, which was redundant in some cases
(`timestamp_field`, `event_category_field`), and invalid in the case of
`tiebreaker_field`. You can see here how the `tiebreaker_field` must be
changed to make the query valid:

https://github.com/elastic/kibana/assets/657252/556bc998-c147-4825-8bde-5d8d03873e75

~~I opted to remove these default values, as well as the defaulting
behavior from the search strategy call (see
10a47fd). While these values may be
persisted in existing saved timelines, this makes the EQL workflow much
cleaner for new users, and those users with persisted EQL options can
now modify and validate those options on the fly.~~

#### Steps to Review

For @elastic/kibana-visualizations and/or @elastic/kibana-data-discovery
: I've added a few tests to the EQL search strategy that document
existing behavior RE EQL option fields. Nothing about the search
strategy itself was changed. If you think the tests aren't useful, I'm
happy to delete them.

1. Create an index without a `@timestamp` field, and insert some data:
    <details>
    <summary>Dev Tools</summary>

       PUT timestamp/
       {
         "mappings": {
           "dynamic": "strict",
           "properties": {
             "timestamp": {
               "type": "date",
               "format": "epoch_second"
             },
             "locale": {
               "type": "keyword"
             },
             "created_at": {
               "type": "date",
               "format": "epoch_second"
             },
             "event": {
               "properties": {
                 "category": {
                   "ignore_above": 1024,
                   "type": "keyword"
                 }
               }
             }
           }
         }
       }

       PUT timestamp/_doc/4
       {
         "timestamp": 1710211630,
         "locale": "es",
         "created_at": 1710211630,
         "event.category": "configuration"
       }
    </details>
2. Create a new EQL detection rule, and choose `timestamp` as the index
pattern
3. Type `any where true` as the query, and observe a validation error
(`Found 1 problem line -1:-1: Unknown column [@timestamp]`)
4. Open EQL Settings popover, and select `timestamp` from the `Timestamp
field` dropdown
5. Observe that the query has been revalidated, and found valid.

### New Behavior:

https://github.com/elastic/kibana/assets/657252/22a63363-d597-4dfd-8f9a-647f4084ec0e

### Related Issues
* Addresses elastic#158326

### Checklist

Delete any items that are not applicable to this PR.

- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit 5be91c9)
@rylnd
Copy link
Contributor

rylnd commented Mar 19, 2024

Closed by 5be91c9; backported to 8.13 in #178994.

@rylnd rylnd closed this as completed Mar 19, 2024
kibanamachine added a commit that referenced this issue Mar 19, 2024
@kibanamachine @rylnd
[8.13] [Security Solution] Incorporates EQL options in EQL query vali…
5814aa9 
…dation on both Rule Creation and Timeline (#178468) (#178994)

# Backport

This will backport the following commits from `main` to `8.13`:
- [[Security Solution] Incorporates EQL options in EQL query validation
on both Rule Creation and Timeline
(#178468)](#178468)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Ryland
Herrick","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-03-19T17:52:35Z","message":"[Security
Solution] Incorporates EQL options in EQL query validation on both Rule
Creation and Timeline (#178468)\n\n## Summary\r\n\r\nThis PR updates the
Detection Rule Creation and Timeline forms to\r\naccount for the new(er)
EQL options fields (`timestamp_field`,\r\n`event_category_field`, and
`tiebreaker_field`) when validating the EQL\r\nquery. While the rule
query and timeline query (respectively) would\r\ncorrectly persist and
use these options, they were unused during EQL\r\nvalidation, meaning
that in certain situations it was impossible to\r\nproduce a \"valid\"
(from the perspective of the form) EQL query if your\r\ndata
necessitated one of those options
(see\r\nhttps://github.com//issues/158326 for more
details).\r\n\r\n### Changes\r\nThe above was accomplished by:\r\n\r\n1.
Adding a hidden form field to the UI for `eqlOptions`\r\n* A lack of a
field component meant that `formData` never held any value\r\nfor this
field\r\n* This meant that the form-based validations (i.e.
`eqlValidator`) would\r\nnot have access to `eqlOptions`\r\n2. Updating
the `onOptionsChange` hook to additionally inform the form\r\nof changes
to `eqlOptions`\r\n* This allows the form to have updated values for
`eqlOptions` with\r\nminimal changes\r\n* Not ideal from a data-flow
perspective, since the field changes are\r\nupdating two copies of
`eqlOptions`. However, we're only using the \"form\r\ncopy\" for the
validation call, and everything else works as it did\r\nbefore.\r\n\r\n
\r\n### Additional changes\r\n__Update__: I've reverted the described
changes to the default timeline\r\nvalues below, and moved that to a
[separate\r\nissue](https://github.com/elastic/security-team/issues/8892).\r\n\r\nIn
the course of performing the above tasks for timeline, I found
the\r\nbehavior there to be slightly different: we were specifying
default\r\nvalues for these fields, which was redundant in some
cases\r\n(`timestamp_field`, `event_category_field`), and invalid in the
case of\r\n`tiebreaker_field`. You can see here how the
`tiebreaker_field` must be\r\nchanged to make the query
valid:\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/657252/556bc998-c147-4825-8bde-5d8d03873e75\r\n\r\n\r\n~~I
opted to remove these default values, as well as the
defaulting\r\nbehavior from the search strategy call
(see\r\n10a47fdaba5860a44638aada78c0cf8c37b2c580). While these values
may be\r\npersisted in existing saved timelines, this makes the EQL
workflow much\r\ncleaner for new users, and those users with persisted
EQL options can\r\nnow modify and validate those options on the
fly.~~\r\n\r\n#### Steps to Review\r\n\r\nFor
@elastic/kibana-visualizations and/or
@elastic/kibana-data-discovery\r\n: I've added a few tests to the EQL
search strategy that document\r\nexisting behavior RE EQL option fields.
Nothing about the search\r\nstrategy itself was changed. If you think
the tests aren't useful, I'm\r\nhappy to delete them.\r\n\r\n1. Create
an index without a `@timestamp` field, and insert some data:\r\n
<details>\r\n <summary>Dev Tools</summary>\r\n\r\n PUT timestamp/\r\n
{\r\n \"mappings\": {\r\n \"dynamic\": \"strict\",\r\n \"properties\":
{\r\n \"timestamp\": {\r\n \"type\": \"date\",\r\n \"format\":
\"epoch_second\"\r\n },\r\n \"locale\": {\r\n \"type\": \"keyword\"\r\n
},\r\n \"created_at\": {\r\n \"type\": \"date\",\r\n \"format\":
\"epoch_second\"\r\n },\r\n \"event\": {\r\n \"properties\": {\r\n
\"category\": {\r\n \"ignore_above\": 1024,\r\n \"type\":
\"keyword\"\r\n }\r\n }\r\n }\r\n }\r\n }\r\n }\r\n\r\n PUT
timestamp/_doc/4\r\n {\r\n \"timestamp\": 1710211630,\r\n \"locale\":
\"es\",\r\n \"created_at\": 1710211630,\r\n \"event.category\":
\"configuration\"\r\n }\r\n </details>\r\n2. Create a new EQL detection
rule, and choose `timestamp` as the index\r\npattern\r\n3. Type `any
where true` as the query, and observe a validation error\r\n(`Found 1
problem line -1:-1: Unknown column [@timestamp]`)\r\n4. Open EQL
Settings popover, and select `timestamp` from the `Timestamp\r\nfield`
dropdown\r\n5. Observe that the query has been revalidated, and found
valid.\r\n\r\n\r\n### New
Behavior:\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/657252/22a63363-d597-4dfd-8f9a-647f4084ec0e\r\n\r\n\r\n\r\n###
Related Issues\r\n* Addresses
https://github.com/elastic/kibana/issues/158326\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n\r\n### For maintainers\r\n\r\n- [x]
This was checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"5be91c94b1c7e6b1aebd5fe75c86e2bc17c90ad0","branchLabelMapping":{"^v8.14.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Feature:Timeline","Feature:Event
Correlation (EQL) Rule","Feature:Rule
Creation","backport:prev-minor","Team:Detection
Engine","v8.14.0"],"title":"[Security Solution] Incorporates EQL options
in EQL query validation on both Rule Creation and
Timeline","number":178468,"url":"https://github.com/elastic/kibana/pull/178468","mergeCommit":{"message":"[Security
Solution] Incorporates EQL options in EQL query validation on both Rule
Creation and Timeline (#178468)\n\n## Summary\r\n\r\nThis PR updates the
Detection Rule Creation and Timeline forms to\r\naccount for the new(er)
EQL options fields (`timestamp_field`,\r\n`event_category_field`, and
`tiebreaker_field`) when validating the EQL\r\nquery. While the rule
query and timeline query (respectively) would\r\ncorrectly persist and
use these options, they were unused during EQL\r\nvalidation, meaning
that in certain situations it was impossible to\r\nproduce a \"valid\"
(from the perspective of the form) EQL query if your\r\ndata
necessitated one of those options
(see\r\nhttps://github.com//issues/158326 for more
details).\r\n\r\n### Changes\r\nThe above was accomplished by:\r\n\r\n1.
Adding a hidden form field to the UI for `eqlOptions`\r\n* A lack of a
field component meant that `formData` never held any value\r\nfor this
field\r\n* This meant that the form-based validations (i.e.
`eqlValidator`) would\r\nnot have access to `eqlOptions`\r\n2. Updating
the `onOptionsChange` hook to additionally inform the form\r\nof changes
to `eqlOptions`\r\n* This allows the form to have updated values for
`eqlOptions` with\r\nminimal changes\r\n* Not ideal from a data-flow
perspective, since the field changes are\r\nupdating two copies of
`eqlOptions`. However, we're only using the \"form\r\ncopy\" for the
validation call, and everything else works as it did\r\nbefore.\r\n\r\n
\r\n### Additional changes\r\n__Update__: I've reverted the described
changes to the default timeline\r\nvalues below, and moved that to a
[separate\r\nissue](https://github.com/elastic/security-team/issues/8892).\r\n\r\nIn
the course of performing the above tasks for timeline, I found
the\r\nbehavior there to be slightly different: we were specifying
default\r\nvalues for these fields, which was redundant in some
cases\r\n(`timestamp_field`, `event_category_field`), and invalid in the
case of\r\n`tiebreaker_field`. You can see here how the
`tiebreaker_field` must be\r\nchanged to make the query
valid:\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/657252/556bc998-c147-4825-8bde-5d8d03873e75\r\n\r\n\r\n~~I
opted to remove these default values, as well as the
defaulting\r\nbehavior from the search strategy call
(see\r\n10a47fdaba5860a44638aada78c0cf8c37b2c580). While these values
may be\r\npersisted in existing saved timelines, this makes the EQL
workflow much\r\ncleaner for new users, and those users with persisted
EQL options can\r\nnow modify and validate those options on the
fly.~~\r\n\r\n#### Steps to Review\r\n\r\nFor
@elastic/kibana-visualizations and/or
@elastic/kibana-data-discovery\r\n: I've added a few tests to the EQL
search strategy that document\r\nexisting behavior RE EQL option fields.
Nothing about the search\r\nstrategy itself was changed. If you think
the tests aren't useful, I'm\r\nhappy to delete them.\r\n\r\n1. Create
an index without a `@timestamp` field, and insert some data:\r\n
<details>\r\n <summary>Dev Tools</summary>\r\n\r\n PUT timestamp/\r\n
{\r\n \"mappings\": {\r\n \"dynamic\": \"strict\",\r\n \"properties\":
{\r\n \"timestamp\": {\r\n \"type\": \"date\",\r\n \"format\":
\"epoch_second\"\r\n },\r\n \"locale\": {\r\n \"type\": \"keyword\"\r\n
},\r\n \"created_at\": {\r\n \"type\": \"date\",\r\n \"format\":
\"epoch_second\"\r\n },\r\n \"event\": {\r\n \"properties\": {\r\n
\"category\": {\r\n \"ignore_above\": 1024,\r\n \"type\":
\"keyword\"\r\n }\r\n }\r\n }\r\n }\r\n }\r\n }\r\n\r\n PUT
timestamp/_doc/4\r\n {\r\n \"timestamp\": 1710211630,\r\n \"locale\":
\"es\",\r\n \"created_at\": 1710211630,\r\n \"event.category\":
\"configuration\"\r\n }\r\n </details>\r\n2. Create a new EQL detection
rule, and choose `timestamp` as the index\r\npattern\r\n3. Type `any
where true` as the query, and observe a validation error\r\n(`Found 1
problem line -1:-1: Unknown column [@timestamp]`)\r\n4. Open EQL
Settings popover, and select `timestamp` from the `Timestamp\r\nfield`
dropdown\r\n5. Observe that the query has been revalidated, and found
valid.\r\n\r\n\r\n### New
Behavior:\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/657252/22a63363-d597-4dfd-8f9a-647f4084ec0e\r\n\r\n\r\n\r\n###
Related Issues\r\n* Addresses
https://github.com/elastic/kibana/issues/158326\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n\r\n### For maintainers\r\n\r\n- [x]
This was checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"5be91c94b1c7e6b1aebd5fe75c86e2bc17c90ad0"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.14.0","branchLabelMappingKey":"^v8.14.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/178468","number":178468,"mergeCommit":{"message":"[Security
Solution] Incorporates EQL options in EQL query validation on both Rule
Creation and Timeline (#178468)\n\n## Summary\r\n\r\nThis PR updates the
Detection Rule Creation and Timeline forms to\r\naccount for the new(er)
EQL options fields (`timestamp_field`,\r\n`event_category_field`, and
`tiebreaker_field`) when validating the EQL\r\nquery. While the rule
query and timeline query (respectively) would\r\ncorrectly persist and
use these options, they were unused during EQL\r\nvalidation, meaning
that in certain situations it was impossible to\r\nproduce a \"valid\"
(from the perspective of the form) EQL query if your\r\ndata
necessitated one of those options
(see\r\nhttps://github.com//issues/158326 for more
details).\r\n\r\n### Changes\r\nThe above was accomplished by:\r\n\r\n1.
Adding a hidden form field to the UI for `eqlOptions`\r\n* A lack of a
field component meant that `formData` never held any value\r\nfor this
field\r\n* This meant that the form-based validations (i.e.
`eqlValidator`) would\r\nnot have access to `eqlOptions`\r\n2. Updating
the `onOptionsChange` hook to additionally inform the form\r\nof changes
to `eqlOptions`\r\n* This allows the form to have updated values for
`eqlOptions` with\r\nminimal changes\r\n* Not ideal from a data-flow
perspective, since the field changes are\r\nupdating two copies of
`eqlOptions`. However, we're only using the \"form\r\ncopy\" for the
validation call, and everything else works as it did\r\nbefore.\r\n\r\n
\r\n### Additional changes\r\n__Update__: I've reverted the described
changes to the default timeline\r\nvalues below, and moved that to a
[separate\r\nissue](https://github.com/elastic/security-team/issues/8892).\r\n\r\nIn
the course of performing the above tasks for timeline, I found
the\r\nbehavior there to be slightly different: we were specifying
default\r\nvalues for these fields, which was redundant in some
cases\r\n(`timestamp_field`, `event_category_field`), and invalid in the
case of\r\n`tiebreaker_field`. You can see here how the
`tiebreaker_field` must be\r\nchanged to make the query
valid:\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/657252/556bc998-c147-4825-8bde-5d8d03873e75\r\n\r\n\r\n~~I
opted to remove these default values, as well as the
defaulting\r\nbehavior from the search strategy call
(see\r\n10a47fdaba5860a44638aada78c0cf8c37b2c580). While these values
may be\r\npersisted in existing saved timelines, this makes the EQL
workflow much\r\ncleaner for new users, and those users with persisted
EQL options can\r\nnow modify and validate those options on the
fly.~~\r\n\r\n#### Steps to Review\r\n\r\nFor
@elastic/kibana-visualizations and/or
@elastic/kibana-data-discovery\r\n: I've added a few tests to the EQL
search strategy that document\r\nexisting behavior RE EQL option fields.
Nothing about the search\r\nstrategy itself was changed. If you think
the tests aren't useful, I'm\r\nhappy to delete them.\r\n\r\n1. Create
an index without a `@timestamp` field, and insert some data:\r\n
<details>\r\n <summary>Dev Tools</summary>\r\n\r\n PUT timestamp/\r\n
{\r\n \"mappings\": {\r\n \"dynamic\": \"strict\",\r\n \"properties\":
{\r\n \"timestamp\": {\r\n \"type\": \"date\",\r\n \"format\":
\"epoch_second\"\r\n },\r\n \"locale\": {\r\n \"type\": \"keyword\"\r\n
},\r\n \"created_at\": {\r\n \"type\": \"date\",\r\n \"format\":
\"epoch_second\"\r\n },\r\n \"event\": {\r\n \"properties\": {\r\n
\"category\": {\r\n \"ignore_above\": 1024,\r\n \"type\":
\"keyword\"\r\n }\r\n }\r\n }\r\n }\r\n }\r\n }\r\n\r\n PUT
timestamp/_doc/4\r\n {\r\n \"timestamp\": 1710211630,\r\n \"locale\":
\"es\",\r\n \"created_at\": 1710211630,\r\n \"event.category\":
\"configuration\"\r\n }\r\n </details>\r\n2. Create a new EQL detection
rule, and choose `timestamp` as the index\r\npattern\r\n3. Type `any
where true` as the query, and observe a validation error\r\n(`Found 1
problem line -1:-1: Unknown column [@timestamp]`)\r\n4. Open EQL
Settings popover, and select `timestamp` from the `Timestamp\r\nfield`
dropdown\r\n5. Observe that the query has been revalidated, and found
valid.\r\n\r\n\r\n### New
Behavior:\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/657252/22a63363-d597-4dfd-8f9a-647f4084ec0e\r\n\r\n\r\n\r\n###
Related Issues\r\n* Addresses
https://github.com/elastic/kibana/issues/158326\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n\r\n### For maintainers\r\n\r\n- [x]
This was checked for breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"5be91c94b1c7e6b1aebd5fe75c86e2bc17c90ad0"}}]}]
BACKPORT-->

---------

Co-authored-by: Ryland Herrick <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rylnd rylnd

@kqualters-elastic kqualters-elastic

Labels
bug Fixes for quality problems that affect the customer experience consider-next Feature:Event Correlation (EQL) Rule Security Solution Event Correlation (EQL) rule type impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. Team:Detection Engine Security Solution Detection Engine Area Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Investigations Security Solution Investigations Team Team:Threat Hunting Security Solution Threat Hunting Team
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@rylnd @yctercero @elasticmachine @MadameSheema @marshallmain @kqualters-elastic