add_process_metadata: enrich process info with process owner (#21068) #21111

dddpaul · 2020-09-16T05:26:36Z

Type: Enhancement

What does this PR do?

Enrich process metadata with process owner info.

Why is it important?

It's quite useful to know process owner, to control traffic, for example.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

[ ]

How to test this PR locally

Run packetbeat with simply config:

processors:
  - add_process_metadata:
      match_pids: [process.pid]

Exec some curl http://example.comfrom commandline
Flow type event will be enriched with process.username

Related issues

Closes Packetbeat: enrich process info with process owner #21068

Use cases

Screenshots

Logs

elasticmachine · 2020-09-16T05:26:37Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

elasticmachine · 2020-09-16T05:26:37Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

elasticmachine · 2020-09-16T05:27:42Z

Pinging @elastic/siem (Team:SIEM)

elasticmachine · 2020-09-16T05:32:59Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2021-07-15T13:44:42.696+0000
Duration: 135 min 22 sec
Commit: 06aa7b2

Test stats 🧪

Test	Results
Failed	0
Passed	49269
Skipped	5396
Total	54665

Trends 🧪

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	49269
Skipped	5396
Total	54665

adriansr · 2020-09-21T16:49:47Z

jenkins run the tests please

adriansr

Thanks for your contribution!

In addition to the comments I left, I think it's necessary to change the title/description of this PR. This isn't actually changing anything in Packetbeat, but updating the add_process_metadata processor to add an username field.

Also, it needs an entry in the CHANGELOG.next.asciidoc file, under Added > Affecting all Beats, so that users know about the new field.

libbeat/processors/add_process_metadata/add_process_metadata.go

libbeat/processors/add_process_metadata/gosysinfo_provider.go

dddpaul · 2020-10-29T19:19:33Z

Hi. I do not understand, what's next requirements? I've resolved all issues.

dddpaul · 2020-11-06T08:22:09Z

@adriansr Hi, could you review again please?

adriansr · 2020-11-06T11:42:26Z

jenkins run tests

elasticmachine · 2020-11-06T13:03:05Z

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	16400
Skipped	1342
Total	17742

adriansr

Apart from the CHANGELOG issue, we're discussing a possible rename of the field process.username to align with the Elastic Common Schema.

adriansr · 2020-11-09T12:15:51Z

CHANGELOG.next.asciidoc

@@ -21,6 +21,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 - Remove the deprecated `xpack.monitoring.*` settings. Going forward only `monitoring.*` settings may be used. {issue}9424[9424] {pull}18608[18608]
 - Added `certificate` TLS verification mode to ignore server name mismatch. {issue}12283[12283] {pull}20293[20293]
 - Autodiscover doesn't generate any configuration when a variable is missing. Previously it generated an incomplete configuration. {pull}20898[20898]
+- `AddProcessMetadata` processor enrich process information with owner name. {issue}21068[21068] {pull}21111[21111]


This is in the wrong section, Breaking changes. It should be under Added > Affecting All Beats.

@adriansr I've placed this line to proper section.

What do you think is the proper field name for process owner? The options as I can see are:
process.username
process.owner
process.owner_name

Or process.owner should be an object with it's own fields?
process.owner.uid
process.owner.name
...

@adriansr ?

Hi @dddpaul, we're still trying to figure out what makes more sense for this use case.

At some point, process.user.* made it into ECS, but then it was reverted. See this comment.
So to align with ECS, we could do two things:

Add user information to the top-level user object.

Add a custom object under process. For example owner.

The first makes more sense, but at the same time is more risky for a processor, as this will cause the processor execution to fail if the input document already contains a user.name or user.id. If overwrite_keys is set, then it will silently overwrite those keys, potentially breaking existing pipelines.

As a general purpose processor I think it lacks the context to know if setting a ^user is the correct thing to do. So setting a key(s) under process is the safest thing to do and what I would most expect as a user.

Another alternative would be to offer an addition target_owner config option that indicates where (or if) process owner info is added to the event. The registered_domain and translate_sid processors take an approach like this.

dddpaul · 2020-12-06T07:07:51Z

Hi, @adriansr what's next steps?

elasticmachine · 2021-01-13T02:07:02Z

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

adriansr · 2021-07-09T13:41:16Z

Hi @dddpaul , I know this has been open for a long time, sorry about that. We are reviewing it.

mergify · 2021-07-15T08:38:24Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b feat/process_owner upstream/feat/process_owner
git merge upstream/master
git push upstream feat/process_owner

…mpty owner name. Update changelog. (elastic#21068)

adriansr · 2021-07-15T13:44:28Z

/test

andrewkroh

LGTM

* master: Forward port 7.13.4 to master (elastic#26971) Use MustAddMetricSet in all metricsets (elastic#26907) add_process_metadata: enrich process info with process owner (elastic#21068) (elastic#21111) Use aws sdk paginator for FilterLogEvents and GetMetricData (elastic#26852) [Filebeat] Allow - for source IP for AWS S3 Access pipeline (elastic#26940) Increase timeout to 30secs (elastic#26841) Add Cluster filter on Kubernetes Overview ECS dashboard (elastic#26919)

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Sep 16, 2020

andresrc added the Team:SIEM label Sep 16, 2020

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Sep 16, 2020

adriansr self-requested a review September 16, 2020 10:09

adriansr suggested changes Sep 22, 2020

View reviewed changes

libbeat/processors/add_process_metadata/add_process_metadata.go Outdated Show resolved Hide resolved

libbeat/processors/add_process_metadata/gosysinfo_provider.go Outdated Show resolved Hide resolved

dddpaul changed the title ~~Packetbeat: enrich process info with process owner (#21068)~~ add_process_metadata: enrich process info with process owner (#21068) Sep 28, 2020

dddpaul requested a review from adriansr October 8, 2020 18:37

adriansr suggested changes Nov 9, 2020

View reviewed changes

andrewkroh added Team:Security-External Integrations and removed Team:SIEM labels Jan 13, 2021

andrewkroh assigned adriansr May 3, 2021

dddpaul requested a review from a team as a code owner July 15, 2021 08:37

dddpaul and others added 5 commits July 15, 2021 11:53

Packetbeat: enrich process info with process owner (elastic#21068)

2c2c4ee

Do not fail on getting process owner errors. Do not enrich event on e…

9c01c3d

…mpty owner name. Update changelog. (elastic#21068)

Move changelog entry to proper place

e2148ae

Populate process.owner.{name,pid}

5d9855a

Make update

06b4af0

Correct changelog duplication

06aa7b2

adriansr force-pushed the feat/process_owner branch from fb82887 to 06aa7b2 Compare July 15, 2021 10:35

andrewkroh approved these changes Jul 19, 2021

View reviewed changes

adriansr approved these changes Jul 19, 2021

View reviewed changes

adriansr merged commit 16e2989 into elastic:master Jul 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add_process_metadata: enrich process info with process owner (#21068) #21111

add_process_metadata: enrich process info with process owner (#21068) #21111

dddpaul commented Sep 16, 2020 •

edited

Loading

elasticmachine commented Sep 16, 2020

elasticmachine commented Sep 16, 2020

elasticmachine commented Sep 16, 2020

elasticmachine commented Sep 16, 2020 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

Trends 🧪

Test stats 🧪

adriansr commented Sep 21, 2020

adriansr left a comment •

edited

Loading

dddpaul commented Oct 29, 2020 •

edited

Loading

dddpaul commented Nov 6, 2020

adriansr commented Nov 6, 2020

elasticmachine commented Nov 6, 2020

Test stats 🧪

adriansr left a comment •

edited

Loading

adriansr Nov 9, 2020

dddpaul Nov 10, 2020 •

edited

Loading

dddpaul Nov 10, 2020

dddpaul Nov 25, 2020

adriansr Jul 12, 2021

andrewkroh Jul 12, 2021

dddpaul commented Dec 6, 2020

elasticmachine commented Jan 13, 2021

adriansr commented Jul 9, 2021

mergify bot commented Jul 15, 2021

adriansr commented Jul 15, 2021

andrewkroh left a comment

add_process_metadata: enrich process info with process owner (#21068) #21111

add_process_metadata: enrich process info with process owner (#21068) #21111

Conversation

dddpaul commented Sep 16, 2020 • edited Loading

What does this PR do?

Why is it important?

Checklist

Author's Checklist

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

elasticmachine commented Sep 16, 2020

elasticmachine commented Sep 16, 2020

elasticmachine commented Sep 16, 2020

elasticmachine commented Sep 16, 2020 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

Trends 🧪

💚 Flaky test report

Test stats 🧪

adriansr commented Sep 21, 2020

adriansr left a comment • edited Loading

Choose a reason for hiding this comment

dddpaul commented Oct 29, 2020 • edited Loading

dddpaul commented Nov 6, 2020

adriansr commented Nov 6, 2020

elasticmachine commented Nov 6, 2020

💚 Flaky test report

Test stats 🧪

adriansr left a comment • edited Loading

Choose a reason for hiding this comment

adriansr Nov 9, 2020

Choose a reason for hiding this comment

dddpaul Nov 10, 2020 • edited Loading

Choose a reason for hiding this comment

dddpaul Nov 10, 2020

Choose a reason for hiding this comment

dddpaul Nov 25, 2020

Choose a reason for hiding this comment

adriansr Jul 12, 2021

Choose a reason for hiding this comment

andrewkroh Jul 12, 2021

Choose a reason for hiding this comment

dddpaul commented Dec 6, 2020

elasticmachine commented Jan 13, 2021

adriansr commented Jul 9, 2021

mergify bot commented Jul 15, 2021

adriansr commented Jul 15, 2021

andrewkroh left a comment

Choose a reason for hiding this comment

dddpaul commented Sep 16, 2020 •

edited

Loading

elasticmachine commented Sep 16, 2020 •

edited by jenkins-beats-ci bot

Loading

adriansr left a comment •

edited

Loading

dddpaul commented Oct 29, 2020 •

edited

Loading

adriansr left a comment •

edited

Loading

dddpaul Nov 10, 2020 •

edited

Loading