Skip to content

x-pack/auditbeat/module/system/socket: get full length path and arg from /proc when not available from kprobe#29410

Merged
efd6 merged 3 commits intoelastic:masterfrom
efd6:auditd/pathlength
Jan 4, 2022
Merged

x-pack/auditbeat/module/system/socket: get full length path and arg from /proc when not available from kprobe#29410
efd6 merged 3 commits intoelastic:masterfrom
efd6:auditd/pathlength

Conversation

@efd6
Copy link
Copy Markdown
Contributor

@efd6 efd6 commented Dec 13, 2021 •
edited
Loading

What does this PR do?

This uses path and arg information from /proc in cases where the kprobe details are truncated.

Why is it important?

Currently filepaths, executable names and argument lists may be truncated due to kernel limitations.

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

No specific recommendation.

How to test this PR locally

Standard testing on linux.

Related issues

Use cases

N/A

Screenshots

N/A

Logs

N/A

@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Dec 13, 2021
@efd6 efd6 force-pushed the auditd/pathlength branch from 24541e3 to 7e6b8fe Compare December 13, 2021 20:53
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Dec 13, 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: 2022-01-03T23:53:30.536+0000

  • Duration: 72 min 18 sec

  • Commit: dc3d5b8

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!)

@efd6 efd6 force-pushed the auditd/pathlength branch 2 times, most recently from 864276f to 1580ebe Compare December 14, 2021 01:28
@efd6
Copy link
Copy Markdown
Contributor Author

efd6 commented Dec 14, 2021

/test

@efd6 efd6 force-pushed the auditd/pathlength branch 2 times, most recently from f89c00b to 58d387c Compare December 14, 2021 08:26
@efd6
x-pack/auditbeat/module/system/socket: get full length path and args …
ec35892 
…from /proc when not available from kprobe

Also use first arg from sysinfo.Processes in place of Name to avoid process name
truncation.
@efd6 efd6 force-pushed the auditd/pathlength branch from 99bd2e6 to ec35892 Compare December 14, 2021 11:31
@efd6 efd6 marked this pull request as ready for review December 14, 2021 12:27
@efd6 efd6 requested a review from a team as a code owner December 14, 2021 12:27
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Dec 14, 2021

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

@efd6
Copy link
Copy Markdown
Contributor Author

efd6 commented Dec 17, 2021

@adriansr I'm wondering if we could (in a separate PR) also add container ID enrichment here to address #22238. To avoid unwanted work on non-containerised systems we would check during init whether we are in a container and only try to get the ID if we are. This should alleviate the timing issues with deferring the enrichment until later without too much additional cost in the general case.

@adriansr
Copy link
Copy Markdown
Contributor

adriansr commented Dec 20, 2021

@efd6 I think it's worth to investigate the container ID enrichment in a separate PR, as you said. I think there's a few issues to solve, afaik the add_process_metadata processor won't react well to having a pre-populated container.id field (it will either overwrite it or fail completely).

adriansr
adriansr approved these changes Dec 20, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@adriansr adriansr left a comment

Choose a reason for hiding this comment

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

LGTM

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Dec 20, 2021

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 auditd/pathlength upstream/auditd/pathlength
git merge upstream/master
git push upstream auditd/pathlength

@efd6 efd6 mentioned this pull request Jan 3, 2022
Closed
@efd6 efd6 merged commit d46bb5f into elastic:master Jan 4, 2022
mergify bot pushed a commit that referenced this pull request Jan 4, 2022
@efd6
x-pack/auditbeat/module/system/socket: get full length path and arg f…
369a374 
…rom /proc when not available from kprobe (#29410)

Also use first arg from sysinfo.Processes in place of Name to avoid process name
truncation.

(cherry picked from commit d46bb5f)
@mergify mergify bot mentioned this pull request Jan 4, 2022
Merged
efd6 added a commit that referenced this pull request Jan 4, 2022
@mergify @efd6
x-pack/auditbeat/module/system/socket: get full length path and arg f…
f99aa9e 
…rom /proc when not available from kprobe (#29410) (#29668)

Also use first arg from sysinfo.Processes in place of Name to avoid process name
truncation.

(cherry picked from commit d46bb5f)

Co-authored-by: Dan Kortschak <90160302+efd6@users.noreply.github.com>
@kesslerm
Copy link
Copy Markdown
Contributor

kesslerm commented Jan 14, 2022 •
edited by jstrassb
Loading

Is there any chance this can be backported to the 7.x series, too? Many users will not be in a position to upgrade immediately to the 8.x series.

@efd6 efd6 added the backport-7.17 Automated backport to the 7.17 branch with mergify label Jan 24, 2022
@mergify mergify bot mentioned this pull request Jan 24, 2022
Merged
mergify bot pushed a commit that referenced this pull request Jan 24, 2022
@efd6
x-pack/auditbeat/module/system/socket: get full length path and arg f…
015ac0b 
…rom /proc when not available from kprobe (#29410)

Also use first arg from sysinfo.Processes in place of Name to avoid process name
truncation.

(cherry picked from commit d46bb5f)
@efd6
Copy link
Copy Markdown
Contributor Author

efd6 commented Jan 24, 2022

@kesslerm Started.

efd6 added a commit that referenced this pull request Jan 24, 2022
@mergify @efd6
x-pack/auditbeat/module/system/socket: get full length path and arg f…
cd16c9f 
…rom /proc when not available from kprobe (#29410) (#29958)

Also use first arg from sysinfo.Processes in place of Name to avoid process name
truncation.

(cherry picked from commit d46bb5f)

Co-authored-by: Dan Kortschak <90160302+efd6@users.noreply.github.com>
v1v added a commit that referenced this pull request Jan 28, 2022
@v1v
Merge remote-tracking branch 'upstream/7.17' into v1v-patch-1
c28c548 
* upstream/7.17: (30 commits)
  [7.17](backport #29966) Add the Elastic product origin header when talking to Elasticsearch or Kibana. (#30000)
  [Heartbeat] Change size of data on ICMP packet (#29948) (#29978)
  Add clarification about enableing dashboard loading (#29985) (#29989)
  Improve aws-s3 gzip file detection to avoid false negatives (#29969) (#29974)
  ci: docker login step for pulling then pushing (#29960) (#29963)
  x-pack/auditbeat/module/system/socket: get full length path and arg from /proc when not available from kprobe (#29410) (#29958)
  [Automation] Update elastic stack version to 7.17.0-ab4975a2 for testing (#29956)
  [Automation] Update elastic stack version to 7.17.0-1bd58b32 for testing (#29938)
  [7.17](backport #29913) [Metricbeat] gcp.gke: fix overview dashboard (#29914)
  [7.17](backport #29605) Fix annotation enrichment (#29834)
  [Automation] Update elastic stack version to 7.17.0-e1efbe3a for testing (#29922)
  [Automation] Update elastic stack version to 7.17.0-68da5d12 for testing (#29904)
  [7.17][Heartbeat] Defer monitor / ICMP errors to monitor runtime / ES (backport #29413) (#29896)
  Merge pull request from GHSA-rj4h-hqvq-cc6q
  [7.17](backport #29681) Change docker image from CentOS 7 to Ubuntu 20.04 (#29817)
  Fix YAML indentation in `parsers` examples (#29663) (#29894)
  [Automation] Update elastic stack version to 7.17.0-079761a0 for testing (#29864)
  Fix Filebeat dissect processor field tokenization in documentation (#29680) (#29883)
  Enable require_alias for Bulk requests for all actions when target is a write alias (#29879)
  Update Index template loading guide to use the correct endpoint (#29869) (#29877)
  ...
@jalogisch jalogisch mentioned this pull request Aug 8, 2022
Closed
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
@v1v
Merge remote-tracking branch 'upstream/7.17' into v1v-patch-1
82291ab 
* upstream/7.17: (30 commits)
  [7.17](backport elastic#29966) Add the Elastic product origin header when talking to Elasticsearch or Kibana. (elastic#30000)
  [Heartbeat] Change size of data on ICMP packet (elastic#29948) (elastic#29978)
  Add clarification about enableing dashboard loading (elastic#29985) (elastic#29989)
  Improve aws-s3 gzip file detection to avoid false negatives (elastic#29969) (elastic#29974)
  ci: docker login step for pulling then pushing (elastic#29960) (elastic#29963)
  x-pack/auditbeat/module/system/socket: get full length path and arg from /proc when not available from kprobe (elastic#29410) (elastic#29958)
  [Automation] Update elastic stack version to 7.17.0-ab4975a2 for testing (elastic#29956)
  [Automation] Update elastic stack version to 7.17.0-1bd58b32 for testing (elastic#29938)
  [7.17](backport elastic#29913) [Metricbeat] gcp.gke: fix overview dashboard (elastic#29914)
  [7.17](backport elastic#29605) Fix annotation enrichment (elastic#29834)
  [Automation] Update elastic stack version to 7.17.0-e1efbe3a for testing (elastic#29922)
  [Automation] Update elastic stack version to 7.17.0-68da5d12 for testing (elastic#29904)
  [7.17][Heartbeat] Defer monitor / ICMP errors to monitor runtime / ES (backport elastic#29413) (elastic#29896)
  Merge pull request from GHSA-rj4h-hqvq-cc6q
  [7.17](backport elastic#29681) Change docker image from CentOS 7 to Ubuntu 20.04 (elastic#29817)
  Fix YAML indentation in `parsers` examples (elastic#29663) (elastic#29894)
  [Automation] Update elastic stack version to 7.17.0-079761a0 for testing (elastic#29864)
  Fix Filebeat dissect processor field tokenization in documentation (elastic#29680) (elastic#29883)
  Enable require_alias for Bulk requests for all actions when target is a write alias (elastic#29879)
  Update Index template loading guide to use the correct endpoint (elastic#29869) (elastic#29877)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@adriansr adriansr adriansr approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

8.1-candidate backport-7.17 Automated backport to the 7.17 branch with mergify backport-v8.0.0 Automated backport with mergify bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Auditbeat: socket reports truncated process name

4 participants

@efd6 @elasticmachine @adriansr @kesslerm