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.

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

[ML] Adds Metadata and Discovery Analysis Jobs to Security Integration #76023

Merged
merged 13 commits into from
Sep 10, 2020

Conversation

blaklaybul
Copy link
Contributor

Summary

This PR contains a suite of new ML job for the Security solution's anomaly detection integration that offers new analyses to enable threat detection on metadata services, system and network discovery processes, compiler events, and processes invoked by sudo.

In total, there are 12 new jobs:

  • siem_auditbeat/ml/linux_rare_kernel_module_arguments
  • siem_auditbeat/ml/linux_rare_metadata_process
  • siem_auditbeat/ml/linux_rare_metadata_user
  • siem_auditbeat/ml/linux_rare_user_compiler
  • siem_auditbeat/ml/linux_rare_sudo_user
  • siem_auditbeat/ml/linux_network_connection_discovery
  • siem_auditbeat/ml/linux_network_configuration_discovery
  • siem_auditbeat/ml/linux_system_information_discovery
  • siem_auditbeat/ml/linux_system_process_discovery
  • siem_auditbeat/ml/linux_system_user_discovery
  • siem_winlogbeat/ml/windows_rare_metadata_process
  • siem_winlogbeat/ml/windows_rare_metadata_user

modules: siem_auditbeat, siem_winlogbeat

@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@peteharverson peteharverson added the Feature:Anomaly Detection ML anomaly detection label Aug 27, 2020
@pheyos
Copy link
Member

pheyos commented Aug 27, 2020

@blaklaybul in order to make the api integration tests pass, you have to add the new modules to the list of expected modules.

@blaklaybul
Copy link
Contributor Author

@pheyos there are no new modules in this PR, it only updates to existing modules. Not sure why I'm getting this error.

@blaklaybul
Copy link
Contributor Author

@elasticmachine merge upstream

@@ -40,6 +40,46 @@
{
"id": "linux_anomalous_user_name_ecs",
"file": "linux_anomalous_user_name_ecs.json"
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the description for this module still OK, or does it need amending to reflect the new mix of jobs in this module?

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@randomuserid how would you like this to be updated?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see why that description cannot apply to the new Linux jobs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great - leaving it as is

@@ -48,6 +48,14 @@
{
"id": "windows_rare_user_runas_event",
"file": "windows_rare_user_runas_event.json"
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, is the description for the winlogbeat module still ok?

image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could remove the words "process and network" because we have branched out to additional event types

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@randomuserid do you want to update this? This is what a user sees when interacting with the module via the ML ui.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't we leave it alone for now. When we have a decision on multi-index Windows events we can rewrite them then.

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added extra comments around the custom URLs.

@blaklaybul
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested latest changes and all looks good. Just added one comment about the influencers for one of the jobs.

],
"influencers": [
"process.name",
"process.working_directory",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to limit the job to 4 influencers for this linux_rare_kernel_module_arguments job, is it better to use process.title in place of process.working_directory? The Anomaly Explorer won't list process.title as a swim lane 'view by' option or in the left hand Top Influencers list otherwise.

Copy link
Contributor Author

@blaklaybul blaklaybul Sep 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's my understanding that for this analysis, it's valuable to know the location (i.e. process.working_directory) from which the process is being executed. @randomuserid, if we had to choose between swapping either process.name or process.working_directory for process.title as an influencer, which do you think should go? Here's some example data:

"process" : {
            "title" : "/bin/sh -e /usr/share/initramfs-tools/hooks/kmod prereqs",
            "name" : "kmod",
            "working_directory" : "/"
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that if we choose to swap out process.name then the two custom URLs that use process.name should be deleted (unless they also work with process.title?).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Process.name is probably the best one to cut because it will tend to show up in the process.title field & the data feed is limited to a few kernel module related processes by name so process name will always be one of those.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spoke with @randomuserid. process.name is limited to just 4 processes because of the datafeed query, so let's swap it out for process.title in the influencer list. As @peteharverson mentioned, this means we'll need to remove the custom urls that utilize process.name. I'll make the change.

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latest changes LGTM!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

distributable file count

id value diff baseline
default 45501 +24 45477

History

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

@blaklaybul blaklaybul merged commit a55edc9 into elastic:master Sep 10, 2020
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 76023 or prevent reminders by adding the backport:skip label.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 14, 2020
blaklaybul pushed a commit to blaklaybul/kibana that referenced this pull request Sep 14, 2020
elastic#76023)

* adds enhanced winlogbeat module

* adds enhanced auditbeat module

* splits discovery jobs

* fixes winlogbeat manifest

* adds process group

* adds custom urls

* adds by field as influencer

* use process.title as influencer

* updates custom url

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 15, 2020
blaklaybul pushed a commit that referenced this pull request Sep 15, 2020
#76023) (#77383)

* adds enhanced winlogbeat module

* adds enhanced auditbeat module

* splits discovery jobs

* fixes winlogbeat manifest

* adds process group

* adds custom urls

* adds by field as influencer

* use process.title as influencer

* updates custom url

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants