[Security Solutions][Detection Engine] Fixes critical clashing with source indexes that already contain a "signal" field

[FrankHassanabad]

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:

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:

# 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.

{
... 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

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

[FrankHassanabad]

Please don't forget to expand this if you don't see github doing for this PR. This is where newly created signal end to end tests I added are.

[FrankHassanabad]

NOTE: This will not allow indexing and searching of the signal but it will allow "conflicts" meaning if a customer has different mappings of "signal" then this will mix between the two correctly but the downside is the search ability of the original signal is not there.

You can search for other things and then see your original_signal in timeline still though or later manually change this particular mapping and re-index your signals if we get some type of "templating" for customers to change around their signals or if they do it today manually.

[dhurley14]

@garrett @FrankHassanabad Looking at this issue #81874 I'm wondering if we can generalize the object property https://github.com/elastic/kibana/pull/82191/files#diff-34de311aba68a55ea153f8adbbbb4050a634baac8ac6bfd97fa4abe74f9ce7a1R260-R264 in such a way that we could incorporate an entire source document or the piece of the document where there was a conflicting ECS field in the source index and retry a bulk index. This way the rule doesn't fail and it can generate a signal on another bulk index call. Thoughts?

[dhurley14]

Just making a note here: in the future we could re-work this function as a type guard for the event doc.

[dhurley14]

LGTM!!! Appreciated the new e2e tests for working with signals. I think my comment about utilizing the fact that since original_signal field is of object type it could be useful outside of just a clashing / conflicting field of signal and instead could be used for any ECS property that would otherwise prevent a signal from being generated because of mapping conflicts. Looks great thank you for the speedy fix!

[FrankHassanabad]

@devin, for #81874, I think that although we are resolving a customer conflict here at this moment for signal since that is the only signal specific field we are "hijacking" and not technically an ECS field I am ok with doing the complexity.

For all other user fields that are non-ECS fields I don't know if we want to deconflict and remove them/change them as the user who has something conflicting such as using host: keyword instead of an object won't be able to use other parts of the applications when using those non-ECS based fields. This would include the ML based rules and pre-packaged content and detections which rely and use ECS.

I fix this signal clash here since signal is not a ECS reserved field/proposal but rather the one meta object we are utilizing and is not being used within signals on signals (rules) or other types of things for looking at meta data. That day might still come but if it does this code will still work as it "moves" the offending insertion record out of the way and then stamps it with its internal structure which is signal and not really supposed to be used by 3rd party indexes.

I don't know if additional complexity/risk of identifying each and every ECS field that is conflicted is really worth it to be honest or will work out as users will next ask why the prepackaged rules looking for something in host.name or another ECS field is not working for them.

At that point you would then need one "large override/mapping tool" which maps everything from ECS to their non-ECS so the pre-packaged rules work, the tables/map work, etc...

[FrankHassanabad]

@elasticmachine merge upstream

[FrankHassanabad]

@elasticmachine merge upstream

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 3c123a5

Metrics [docs]

✅ unchanged

History

• 💚 Build #85945 succeeded 58e9e99
• 💚 Build #85264 succeeded 6aa7d90
• 💚 Build #85231 succeeded 5034912
• 💚 Build #84957 succeeded f96873b

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