[SIEM][Detection Engine] Adds ecs threat properties to rules

[dhurley14]

Summary

Allows addition of ecs threat properties for mitre attack info to rules / signals.

Summarize your PR. If it involves visual changes include a screenshot or gif.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

- [ ] This was checked for cross-browser compatibility, including a check against IE11
- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
- [ ] Documentation was added for features that require explanation or tutorials

• Unit or functional tests were updated or added to match the most common scenarios
  - [ ] This was checked for keyboard-only and screenreader accessibility

For maintainers

• This was checked for breaking API changes and was labeled appropriately
• This includes a feature addition or change that requires a release note and was labeled appropriately

[elasticmachine]

Pinging @elastic/siem (Team:SIEM)

[FrankHassanabad]

I'd add one or two unit tests to the update tests, we might be getting to the point where we should possibly break this test file up into small test files but doesn't have to be this PR. It could be one later.

[FrankHassanabad]

I think you can just have threats without having to put in optional() since its default state is optional.

[FrankHassanabad]

optional: I would just default this on the way in as an empty array if it's not set like so:

threats: threats.default([])

Will make the typing and code easier. It's at least what I have done for references below and tags above when the user doesn't push anything in, I just default them to []

Update: If you require something something other than an empty then this is great and probably better than the ones I have and I should update mine at some point.

[dhurley14]

Looks like I can default to an empty array and not throw any errors 👍 Will update this too.

[FrankHassanabad]

This is directly from:
https://github.com/elastic/ecs/blob/master/generated/elasticsearch/7/template.json#L1759

And should be threat as this part of the mapping is still part of the copying mechanism.

You don't want to change this to the word threats here.

[FrankHassanabad]

update:
I think really we should try to use a nested mapping here?
https://www.elastic.co/guide/en/elasticsearch/reference/current/nested.html

I haven't used one before and don't know how it works with KQL, etc... but I think that would be the best way to support indexing and KQL.

edit: Most of this below is wrong, I put strike outs where I could:

This part here should be directly from ECS rather than object so we get KQL and indexing support.

So you want this to be:

edit: Ignore this below:

      "threat": {
        "properties": {
          "framework": {
            "type": "keyword"
          },
          "tactic": {
            "properties": {
              "id": {
                "type": "keyword"
              },
              "name": {
                "type": "keyword"
              },
              "reference": {
                "type": "keyword"
              }
            }
          },
          "technique": {
            "properties": {
              "id": {
                "type": "keyword"
              },
              "name": {
                "type": "keyword"
              },
              "reference": {
                "type": "keyword"
              }
            }
          }
        }
      },

edit: This is still a valid thing I think we want to do before shipping:
Before shipping we will probably change this file to be two different parts. One JSON which is directly from the ECS generated mapping as-is and then a TypeScript file that imports it and adds the extra signal mapping in as that will be more maintainable and upgrades should be simpler.

[FrankHassanabad]

Without nesting this seems to work ok, so I am fine with it where it is at unless we see other issues.

[FrankHassanabad]

per comments below and how ECS labels it as threat I would keep it at threat even though yes it is indeed multiple threats.

Actually this should be fine as threats, I updated my comments below. I am wrong here as this is different than the ECS threats, this is a threat from a rule which would probably be a nested object.

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 84dc510

[FrankHassanabad]

Tested with this example:

{
  "rule_id": "rule-threat",
  "risk_score": 1,
  "description": "Detecting root and admin users",
  "interval": "5m",
  "name": "Detect Root/Admin Users",
  "severity": "high",
  "type": "query",
  "from": "now-6m",
  "to": "now",
  "query": "user.name: root or user.name: admin",
  "language": "kuery",
  "references": ["http://www.example.com", "https://ww.example.com"],
  "threats": [
    {
      "framework": "MITRE ATT&CK",
      "tactic": {
        "id": "TA0040",
        "name": "impact",
        "reference": "https://attack.mitre.org/tactics/TA0040/"
      },
      "technique": {
        "id": "T1499",
        "name": "endpoint denial of service",
        "reference": "https://attack.mitre.org/techniques/T1499/"
      }
    },
    {
      "framework": "MITRE ATT&CK",
      "tactic": {
        "id": "T1020",
        "name": "Automated Exfiltration",
        "reference": "https://attack.mitre.org/techniques/T1020/"
      },
      "technique": {
        "id": "T1002",
        "name": "Data Compressed",
        "reference": "https://attack.mitre.org/techniques/T1002/"
      }
    }
  ]
}

and everything works as expected. Optional: just add an example for us to use and test with and I think this is LGTM! 🦃 🦃 🦃 :-)

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: b3045c7