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

[Security Solutions][Detection Engine] indexes with a mapping of signal causes clashes #82148

Closed
FrankHassanabad opened this issue Oct 30, 2020 · 1 comment · Fixed by #82191
Closed

[Security Solutions][Detection Engine] indexes with a mapping of signal causes clashes #82148

FrankHassanabad opened this issue Oct 30, 2020 · 1 comment · Fixed by #82191
Labels
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:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v7.10.1

Comments

@FrankHassanabad
Copy link
Contributor

FrankHassanabad commented Oct 30, 2020 •
edited
Loading

Describe the bug:
If you have a source index value which contains the name signal and run the detection engine you will get an error showing up that says:

Bulk Indexing of signals failed: TypeError: Cannot read property 'some' of undefined name: "Clashing index" id: "ed032294-3dcd-4bb6-b620-3f3861702deb" rule id: "dc4fc7e0-f8b9-4ab3-9557-f370b90acd52" signals index: ".siem-signals-default"

Workaround while this is still "open" is to not use the keyword of signal in the source index but rather a different field and then re-index all of your data.

Kibana/Elasticsearch Stack version:
7.8.0-7.10.0

Server OS version:
any

Browser and Browser OS versions:
any

Elastic Endpoint version:
any

Original install method (e.g. download page, yum, from source, etc.):
directly from mater

Functional Area (e.g. Endpoint management, timelines, resolver, etc.):
detection engine

Steps to reproduce:

  1. Do to dev tools and insert this record
# This causes a clash with us using the name signal.
PUT clashing-index/_doc/1
{
  "@timestamp": "2020-10-28T05:08:53.000Z",
  "signal": 1
}
  1. Create a new detection rule which will find that record
  2. After it runs you will see this error:

Screen Shot 2020-10-30 at 9 26 43 AM

Current behavior:
Does not create a signal and has errors

Expected behavior:
No errors and it runs correctly without requiring people to change their index.

Errors in browser console (if relevant):

Bulk Indexing of signals failed: TypeError: Cannot read property 'some' of undefined name: "Clashing index" id: "ed032294-3dcd-4bb6-b620-3f3861702deb" rule id: "dc4fc7e0-f8b9-4ab3-9557-f370b90acd52" signals index: ".siem-signals-default"

Any additional context (logs, chat logs, magical formulas, etc.):
Abracadabra
@FrankHassanabad FrankHassanabad added bug Fixes for quality problems that affect the customer experience Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. labels Oct 30, 2020
@MadameSheema
Copy link
Member

@spong @peluja1012 can you please help to prioritise this and label the impact? Thanks :)

@FrankHassanabad FrankHassanabad mentioned this issue Nov 2, 2020
Merged
1 task
@FrankHassanabad FrankHassanabad added impact:critical This issue should be addressed immediately due to a critical level of impact on the product. v7.10.1 impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. and removed impact:critical This issue should be addressed immediately due to a critical level of impact on the product. labels Nov 2, 2020
FrankHassanabad added a commit that referenced this issue Nov 6, 2020
@FrankHassanabad
[Security Solutions][Detection Engine] Fixes critical clashing with s…
b6d661f 
…ource indexes that already contain a "signal" field (#82191)

## Summary

Fixes: #82148


We have errors and do not generate a signal when a source index already has utilized and reserved the "signal" field for their own data purposes. This fix is a bit tricky and has one medium sized risk which is we also support "signals generated on top of existing signals". Therefore we have to be careful and do a small runtime detection of the "data shape" of the signal's data type. If it looks like the user is using the "signal" field within their mapping instead of us, we move the customer's signal into "original_signal" inside our "signal" structure we create when we copy their data set when creating a signal.   

To help mitigate the risks associated with this critical bug with regards to breaking signals on top of signals I have:

* This adds unit tests
* This adds end to end tests for testing generating signals including signals on signals to help mitigate risk

The key test for this shape in the PR are in the file:

```
detection_engine/signals/build_event_type_signal.ts
```

like so:
```ts
export const isEventTypeSignal = (doc: BaseSignalHit): boolean => {
  return doc._source.signal?.rule?.id != null && typeof doc._source.signal?.rule?.id === 'string';
};
```
 
Example of what happens when it does a "move" of an existing numeric signal keyword type:

```ts
# This causes a clash with us using the name signal as a numeric.
PUT clashing-index/_doc/1
{
  "@timestamp": "2020-10-28T05:08:53.000Z",
  "signal": 1
}
```

Before, this was an error. With this PR it now will restructure this data like so when creating a signal along with additional signal ancestor information, meta data. I omitted some of the data from the output signal for this example. 

```ts
{
... Other data copied ...
"signal": 
{
    "original_signal": 1 <--- We "move it" here now
    "parents": 
    [
        {
            "id": "BhbXBmkBR346wHgn4PeZ",
            "type": "event",
            "index": "your-index-name",
            "depth": 0
        },
    ],
    "ancestors":
    [
        {
            "id": "BhbXBmkBR346wHgn4PeZ",
            "type": "event",
            "index": "your-index-name",
            "depth": 0
        },
    ],
    "status": "open",
    "depth": 1,
    "parent":
    {
        "id": "BhbXBmkBR346wHgn4PeZ",
        type: "event",
        "index": "your-index-name",
        "depth": 0
    },
    "original_time": "2019-02-19T17:40:03.790Z",
    "original_event": 
    {
        "action": "socket_closed",
        "dataset": "socket",
        "kind": "event",
        "module": "system"
    },
}

```

### 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
FrankHassanabad added a commit to FrankHassanabad/kibana that referenced this issue Nov 6, 2020
@FrankHassanabad
[Security Solutions][Detection Engine] Fixes critical clashing with s…
3e1e65b 
…ource indexes that already contain a "signal" field (elastic#82191)

## Summary

Fixes: elastic#82148


We have errors and do not generate a signal when a source index already has utilized and reserved the "signal" field for their own data purposes. This fix is a bit tricky and has one medium sized risk which is we also support "signals generated on top of existing signals". Therefore we have to be careful and do a small runtime detection of the "data shape" of the signal's data type. If it looks like the user is using the "signal" field within their mapping instead of us, we move the customer's signal into "original_signal" inside our "signal" structure we create when we copy their data set when creating a signal.   

To help mitigate the risks associated with this critical bug with regards to breaking signals on top of signals I have:

* This adds unit tests
* This adds end to end tests for testing generating signals including signals on signals to help mitigate risk

The key test for this shape in the PR are in the file:

```
detection_engine/signals/build_event_type_signal.ts
```

like so:
```ts
export const isEventTypeSignal = (doc: BaseSignalHit): boolean => {
  return doc._source.signal?.rule?.id != null && typeof doc._source.signal?.rule?.id === 'string';
};
```
 
Example of what happens when it does a "move" of an existing numeric signal keyword type:

```ts
# This causes a clash with us using the name signal as a numeric.
PUT clashing-index/_doc/1
{
  "@timestamp": "2020-10-28T05:08:53.000Z",
  "signal": 1
}
```

Before, this was an error. With this PR it now will restructure this data like so when creating a signal along with additional signal ancestor information, meta data. I omitted some of the data from the output signal for this example. 

```ts
{
... Other data copied ...
"signal": 
{
    "original_signal": 1 <--- We "move it" here now
    "parents": 
    [
        {
            "id": "BhbXBmkBR346wHgn4PeZ",
            "type": "event",
            "index": "your-index-name",
            "depth": 0
        },
    ],
    "ancestors":
    [
        {
            "id": "BhbXBmkBR346wHgn4PeZ",
            "type": "event",
            "index": "your-index-name",
            "depth": 0
        },
    ],
    "status": "open",
    "depth": 1,
    "parent":
    {
        "id": "BhbXBmkBR346wHgn4PeZ",
        type: "event",
        "index": "your-index-name",
        "depth": 0
    },
    "original_time": "2019-02-19T17:40:03.790Z",
    "original_event": 
    {
        "action": "socket_closed",
        "dataset": "socket",
        "kind": "event",
        "module": "system"
    },
}

```

### 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
FrankHassanabad added a commit that referenced this issue Nov 6, 2020
@FrankHassanabad
[Security Solutions][Detection Engine] Fixes critical clashing with s…
d51e117 
…ource indexes that already contain a "signal" field (#82191) (#82841)

## Summary

Fixes: #82148


We have errors and do not generate a signal when a source index already has utilized and reserved the "signal" field for their own data purposes. This fix is a bit tricky and has one medium sized risk which is we also support "signals generated on top of existing signals". Therefore we have to be careful and do a small runtime detection of the "data shape" of the signal's data type. If it looks like the user is using the "signal" field within their mapping instead of us, we move the customer's signal into "original_signal" inside our "signal" structure we create when we copy their data set when creating a signal.   

To help mitigate the risks associated with this critical bug with regards to breaking signals on top of signals I have:

* This adds unit tests
* This adds end to end tests for testing generating signals including signals on signals to help mitigate risk

The key test for this shape in the PR are in the file:

```
detection_engine/signals/build_event_type_signal.ts
```

like so:
```ts
export const isEventTypeSignal = (doc: BaseSignalHit): boolean => {
  return doc._source.signal?.rule?.id != null && typeof doc._source.signal?.rule?.id === 'string';
};
```
 
Example of what happens when it does a "move" of an existing numeric signal keyword type:

```ts
# This causes a clash with us using the name signal as a numeric.
PUT clashing-index/_doc/1
{
  "@timestamp": "2020-10-28T05:08:53.000Z",
  "signal": 1
}
```

Before, this was an error. With this PR it now will restructure this data like so when creating a signal along with additional signal ancestor information, meta data. I omitted some of the data from the output signal for this example. 

```ts
{
... Other data copied ...
"signal": 
{
    "original_signal": 1 <--- We "move it" here now
    "parents": 
    [
        {
            "id": "BhbXBmkBR346wHgn4PeZ",
            "type": "event",
            "index": "your-index-name",
            "depth": 0
        },
    ],
    "ancestors":
    [
        {
            "id": "BhbXBmkBR346wHgn4PeZ",
            "type": "event",
            "index": "your-index-name",
            "depth": 0
        },
    ],
    "status": "open",
    "depth": 1,
    "parent":
    {
        "id": "BhbXBmkBR346wHgn4PeZ",
        type: "event",
        "index": "your-index-name",
        "depth": 0
    },
    "original_time": "2019-02-19T17:40:03.790Z",
    "original_event": 
    {
        "action": "socket_closed",
        "dataset": "socket",
        "kind": "event",
        "module": "system"
    },
}

```

### 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
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:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v7.10.1
Projects
None yet
Milestone
No milestone
2 participants
@FrankHassanabad @MadameSheema