Skip to content

Auditbeat: Fix processes misattribution in system/socket dataset#29166

Merged
adriansr merged 8 commits intoelastic:masterfrom
adriansr:ab_test
Dec 2, 2021
Merged

Auditbeat: Fix processes misattribution in system/socket dataset#29166
adriansr merged 8 commits intoelastic:masterfrom
adriansr:ab_test

Conversation

@adriansr
Copy link
Copy Markdown
Contributor

@adriansr adriansr commented Nov 28, 2021 •
edited
Loading

What does this PR do?

This PR adds some fixes to Auditbeat's system/socket dataset in order to improve the process/flow correlation. During testing, the following issues have been identified.

  • Process forks were not tracked. This was an oversight that would trigger async process information fetch
  • Receiving a new socket via accept was not handled properly. This was a major cause of process misattribution as the new flow could be associated to the previous process using the socket.
  • Fixes wrong client/server identification in some scenarios.

Why is it important?

It had been reported that the dataset could provide incorrect process information, associating flows with the wrong process, and could also mismatch the origin of a connection, assigning the server side as the client.

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.

Related issues

Fixes #17165

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Nov 28, 2021
@adriansr adriansr added in progress Pull request is currently in progress. Team:Security-External Integrations labels Nov 28, 2021
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Nov 28, 2021
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Nov 28, 2021

This pull request does not have a backport label. Could you fix it @adriansr? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

@mergify mergify bot added the backport-skip Skip notification from the automated backport with mergify label Nov 28, 2021
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 28, 2021 •
edited by jenkins-beats-ci bot
Loading

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2021-12-01T22:46:07.550+0000

  • Duration: 69 min 41 sec

  • Commit: 183188f

Test stats 🧪

Test Results
Failed 0
Passed 342
Skipped 145
Total 487

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@andrewkroh andrewkroh mentioned this pull request Nov 30, 2021
Closed
7 tasks
@adriansr adriansr changed the title [Draft] Do not merge: Auditbeat socket testing Auditbeat: Fix processes misattribution in system/socket dataset Dec 1, 2021
@adriansr adriansr marked this pull request as ready for review December 1, 2021 13:58
@adriansr adriansr requested a review from a team as a code owner December 1, 2021 13:58
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Dec 1, 2021

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

@adriansr adriansr added Auditbeat bug review backport-v7.16.0 Automated backport with mergify backport-v8.0.0 Automated backport with mergify and removed in progress Pull request is currently in progress. labels Dec 1, 2021
@mergify mergify bot removed the backport-skip Skip notification from the automated backport with mergify label Dec 1, 2021
efd6
efd6 approved these changes Dec 1, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@efd6 efd6 left a comment

Choose a reason for hiding this comment

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

Nice.

x-pack/auditbeat/module/system/socket/state.go Outdated
pid: childPID,
name: parent.name,
path: parent.path,
args: parent.args, // TODO: shallow copy
Copy link
Copy Markdown
Contributor

@efd6 efd6 Dec 1, 2021

Choose a reason for hiding this comment

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

Would this copy be useful? We don't mutate process.args element-wise after creation, so this should be fine the way you have it.

Copy link
Copy Markdown
Contributor Author

@adriansr adriansr Dec 1, 2021

Choose a reason for hiding this comment

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

Done

@adriansr adriansr merged commit fab2197 into elastic:master Dec 2, 2021
mergify bot pushed a commit that referenced this pull request Dec 2, 2021
@adriansr
Auditbeat: Fix processes misattribution in system/socket dataset (#29166
ce98ee2 
)

- Track forked processes
- Make accept() create a new socket
- Improve flow tracking
- Set client/server depending on port number when direction is unknown
- Use kernel's monotonic clock explicitly

(cherry picked from commit fab2197)
@mergify mergify bot mentioned this pull request Dec 2, 2021
Merged
mergify bot pushed a commit that referenced this pull request Dec 2, 2021
@adriansr
Auditbeat: Fix processes misattribution in system/socket dataset (#29166
accd6d9 
)

- Track forked processes
- Make accept() create a new socket
- Improve flow tracking
- Set client/server depending on port number when direction is unknown
- Use kernel's monotonic clock explicitly

(cherry picked from commit fab2197)
@mergify mergify bot mentioned this pull request Dec 2, 2021
Merged
adriansr added a commit that referenced this pull request Dec 2, 2021
@mergify @adriansr
Auditbeat: Fix processes misattribution in system/socket dataset (#29166
8cb4f8e 
) (#29244)

- Track forked processes
- Make accept() create a new socket
- Improve flow tracking
- Set client/server depending on port number when direction is unknown
- Use kernel's monotonic clock explicitly

(cherry picked from commit fab2197)

Co-authored-by: Adrian Serrano <adrisr83@gmail.com>
adriansr added a commit that referenced this pull request Dec 2, 2021
@mergify @adriansr
Auditbeat: Fix processes misattribution in system/socket dataset (#29166
4bcd954 
) (#29243)

- Track forked processes
- Make accept() create a new socket
- Improve flow tracking
- Set client/server depending on port number when direction is unknown
- Use kernel's monotonic clock explicitly

(cherry picked from commit fab2197)

Co-authored-by: Adrian Serrano <adrisr83@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@efd6 efd6 efd6 approved these changes

Assignees

No one assigned

Labels

Auditbeat backport-v7.16.0 Automated backport with mergify backport-v8.0.0 Automated backport with mergify bug review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

auditbeat/socket returns incorrect process information

3 participants

@adriansr @elasticmachine @efd6