[Security Solutions][Detection Engine] Fixes timestamp bugs within source indexes when the formats are not ISO8601 format #101349

FrankHassanabad · 2021-06-03T23:36:44Z

Summary

We have a few bugs where when the source index for detections is not "strict_date_optional_time" it is possible that we will misinterpret the format to be epoch milliseconds when it could be epoch seconds or another ambiguous format or blow up when trying to write out the signals index. This fixes it to where we query for the source index format as an ISO8601 and when we copy the date time format we copy it back out as ISO8601 and insert it into the signal index as ISO8601.

See this gist for more details of how this was accidentally introduced when we added support for runtime fields and the general idea of the fix.

Removes docvalue_field and we now only use fields in detection engine search requests
Splits out the timestamp e2e tests into their own file for timestamps file
Adds more tests to ensure we copy what we expect and we are converting to ISO8601 in the signals
Removes ts-expect-error in a lot of areas including tests and then I fix the types and issues once it is removed.

Checklist

Unit or functional tests were updated or added to match the most common scenarios

…ptional

FrankHassanabad · 2021-06-04T18:31:45Z

x-pack/test/functional/es_archives/security_solution/timestamp_override_4/data.json

@@ -6,7 +6,7 @@
      "message": "hello world 4",
      "@timestamp": "2020-12-16T15:16:18.570Z",
      "event": {
-        "ingested": "2020-12-16T15:16:18.570Z"
+        "ingested": "2020-12-16T16:16:18.570Z"


NOTE: Slight change just to ensure the tests still pass since event.ingested is usually a different value.

FrankHassanabad · 2021-06-04T18:32:06Z

x-pack/test/functional/es_archives/security_solution/timestamp_override_3/mappings.json

@@ -3,6 +3,7 @@
  "value": {
    "index": "myfakeindex-3",
    "mappings": {
+      "dynamic": "strict",


NOTE: Just added this to make sure I didn't add any bugs so it will blow up if it cannot insert the correct data.

kibanamachine · 2021-06-04T20:08:16Z

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

💛 Build #129267 was flaky 02184dc
💚 Build #129259 succeeded b301d84
💚 Build #129181 succeeded e731720
💔 Build #129074 failed b8c57f5

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

cc @FrankHassanabad

dhurley14

Reading through the gist referenced in the PR description my understanding is that the introduction of the field property into the query is overriding the formatting forced by the docvalue_fields which can lead to parsing error bugs. And that by moving the formatted timestamp / timestamp override fields (originally the docvalue_fields) into the field property we are able to continue to ensure proper formatting. This looks great! Thanks for catching this and the extra work around removing the ts-expect-error and the additional integration tests.

LGTM!

TL;DR from the gist:

Old and bad / buggy way

# Old and BAD because formatting doesn't stick
GET packetbeat*/_search
{
  "size": 1,
  "docvalue_fields": [
    {
      "field": "@timestamp",
      "format": "YYYY"
    }],
    "fields": [
      {"field": "*",
      "include_unmapped": "true"
      }
    ]
}

will return 

"@timestamp" : [
   "2021-06-04T21:07:21.951Z"
],

New and not buggy results

GET packetbeat*/_search
{
  "size": 1,
    "fields": [
      {"field": "*",
      "include_unmapped": "true"
      },
      {"field": "@timestamp",
      "format": "YYYY"}
    ]
}

will result in 

"@timestamp" : [
   "2021"
],

dhurley14 · 2021-06-04T21:53:36Z

x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts

@@ -122,6 +119,7 @@ export const buildEventsSearchQuery = ({
          field: '*',
          include_unmapped: true,
        },
+        ...docFields,


dhurley14 · 2021-06-04T21:53:58Z

x-pack/test/detection_engine_api_integration/security_and_spaces/tests/timestamps.ts

@@ -0,0 +1,208 @@
+/*


Thank you for moving these relevant tests into their own file!

dhurley14 · 2021-06-04T21:55:11Z

x-pack/test/detection_engine_api_integration/security_and_spaces/tests/timestamps.ts

+   * partial errors happen correctly
+   */
+  describe('timestamp tests', () => {
+    describe('Signals generated from events with a timestamp in seconds is converted correctly into the forced ISO8601 format when copying', () => {


💥 Tests for this formatting bug. Nice addition.

…urce indexes when the formats are not ISO8601 format (elastic#101349) ## Summary We have a few bugs where when the source index for detections is not `"strict_date_optional_time"` it is possible that we will misinterpret the format to be epoch milliseconds when it could be epoch seconds or another ambiguous format or blow up when trying to write out the signals index. This fixes it to where we query for the source index format as an ISO8601 and when we copy the date time format we copy it back out as ISO8601 and insert it into the signal index as ISO8601. See this [gist](https://gist.github.com/FrankHassanabad/f614ec9762d59cd1129b3269f5bae41c) for more details of how this was accidentally introduced when we added support for runtime fields and the general idea of the fix. * Removes `docvalue_field` and we now only use `fields` in detection engine search requests * Splits out the timestamp e2e tests into their own file for `timestamps` file * Adds more tests to ensure we copy what we expect and we are converting to ISO8601 in the signals * Removes `ts-expect-error` in a lot of areas including tests and then I fix the types and issues once it is removed. ### Checklist - [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

kibanamachine · 2021-06-04T22:22:34Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

…urce indexes when the formats are not ISO8601 format (#101349) (#101440) ## Summary We have a few bugs where when the source index for detections is not `"strict_date_optional_time"` it is possible that we will misinterpret the format to be epoch milliseconds when it could be epoch seconds or another ambiguous format or blow up when trying to write out the signals index. This fixes it to where we query for the source index format as an ISO8601 and when we copy the date time format we copy it back out as ISO8601 and insert it into the signal index as ISO8601. See this [gist](https://gist.github.com/FrankHassanabad/f614ec9762d59cd1129b3269f5bae41c) for more details of how this was accidentally introduced when we added support for runtime fields and the general idea of the fix. * Removes `docvalue_field` and we now only use `fields` in detection engine search requests * Splits out the timestamp e2e tests into their own file for `timestamps` file * Adds more tests to ensure we copy what we expect and we are converting to ISO8601 in the signals * Removes `ts-expect-error` in a lot of areas including tests and then I fix the types and issues once it is removed. ### Checklist - [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 Co-authored-by: Frank Hassanabad <frank.hassanabad@elastic.co>

* master: (90 commits) Fix UI breaks on providing long search keyword in 'Search Box' (elastic#101385) Adds css class to EuiDescriptionListDescription in order to break word on exception details card (elastic#101481) [Lens] Increase timings for drag and drop tests (elastic#101380) [Lens] Fix editor react error on configuration panel (elastic#101367) [Fleet] Move integrations to a separate app (elastic#99848) Fix incorrect message displayed on importing Timeline Templates (elastic#101288) [Cases] RBAC (elastic#95058) [APM] Visual improvements for new APM layout with left navigation (elastic#101360) [master] More precise alerts matching (elastic#99820) [Lens] Value in legend (elastic#101353) Revert "[Reporting] ILM policy for managing reporting indices (elastic#100130)" (elastic#101358) [Discover] Fix header row of data grid in Firefox (elastic#101374) Add link to advanced setting in Discover (elastic#101154) Url service locators (elastic#101045) [Timelion] Update the removal message to mention the exact version (elastic#100994) [Security Solution][Detection Engine] Test cases for alias failure test cases where we don't copy aliases correctly (elastic#101437) [Event Log] Adding `type_id` to saved object array in event log (elastic#100939) [Reporting] Add `location.url` info to console message logs (elastic#101427) [Security Solutions][Detection Engine] Fixes timestamp bugs within source indexes when the formats are not ISO8601 format (elastic#101349) Improve Task Manager instrumentation (elastic#99160) ...

elasticmachine · 2021-06-28T12:59:55Z

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

FrankHassanabad added 2 commits June 3, 2021 10:04

wip

60f6d4a

Fixed query to work with different source timestamps

b8c57f5

FrankHassanabad requested review from delvedor and removed request for delvedor June 4, 2021 13:28

FrankHassanabad self-assigned this Jun 4, 2021

FrankHassanabad added 5 commits June 4, 2021 07:29

Merge branch 'master' into fix-timestamp-source-bug

e731720

Merge branch 'master' into fix-timestamp-source-bug

f68ae9b

Removed optional parameter and fixed tests to operate with it _not_ o…

b301d84

…ptional

Removes another ts-expect-error and fixes the typing around it

02184dc

Added more tests and fixed more types and removed more ts-expect-error

88d2e1e

FrankHassanabad added v8.0.0 v7.14.0 release_note:fix labels Jun 4, 2021

FrankHassanabad changed the title ~~Fix timestamp source bug~~ [Security Solutions][Detection Engine] Fixes timestamp bugs within source indexes when the formats are not ISO8601 format Jun 4, 2021

FrankHassanabad marked this pull request as ready for review June 4, 2021 18:29

FrankHassanabad requested a review from a team as a code owner June 4, 2021 18:29

FrankHassanabad commented Jun 4, 2021

View reviewed changes

FrankHassanabad requested a review from dhurley14 June 4, 2021 20:05

dhurley14 approved these changes Jun 4, 2021

View reviewed changes

FrankHassanabad merged commit e5944a3 into elastic:master Jun 4, 2021

FrankHassanabad added the auto-backport Deprecated - use backport:version if exact versions are needed label Jun 4, 2021

FrankHassanabad deleted the fix-timestamp-source-bug branch June 4, 2021 22:20

kibanamachine mentioned this pull request Jun 4, 2021

[7.x] [Security Solutions][Detection Engine] Fixes timestamp bugs within source indexes when the formats are not ISO8601 format (#101349) #101440

Merged

timroes added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Jun 28, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solutions][Detection Engine] Fixes timestamp bugs within source indexes when the formats are not ISO8601 format #101349

[Security Solutions][Detection Engine] Fixes timestamp bugs within source indexes when the formats are not ISO8601 format #101349

FrankHassanabad commented Jun 3, 2021 •

edited

Loading

FrankHassanabad Jun 4, 2021

FrankHassanabad Jun 4, 2021

kibanamachine commented Jun 4, 2021

dhurley14 left a comment •

edited

Loading

dhurley14 Jun 4, 2021

dhurley14 Jun 4, 2021

dhurley14 Jun 4, 2021

kibanamachine commented Jun 4, 2021

elasticmachine commented Jun 28, 2021

[Security Solutions][Detection Engine] Fixes timestamp bugs within source indexes when the formats are not ISO8601 format #101349

[Security Solutions][Detection Engine] Fixes timestamp bugs within source indexes when the formats are not ISO8601 format #101349

Conversation

FrankHassanabad commented Jun 3, 2021 • edited Loading

Summary

Checklist

FrankHassanabad Jun 4, 2021

Choose a reason for hiding this comment

FrankHassanabad Jun 4, 2021

Choose a reason for hiding this comment

kibanamachine commented Jun 4, 2021

💚 Build Succeeded

Metrics [docs]

History

dhurley14 left a comment • edited Loading

Choose a reason for hiding this comment

dhurley14 Jun 4, 2021

Choose a reason for hiding this comment

dhurley14 Jun 4, 2021

Choose a reason for hiding this comment

dhurley14 Jun 4, 2021

Choose a reason for hiding this comment

kibanamachine commented Jun 4, 2021

💚 Backport successful

elasticmachine commented Jun 28, 2021

FrankHassanabad commented Jun 3, 2021 •

edited

Loading

dhurley14 left a comment •

edited

Loading