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.

Sign up for GitHub

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

Jump to bottom

filebeat/module/auditd: teach kv about quoted spaces #34069

Merged
merged 1 commit into from
Jan 18, 2023
Merged

filebeat/module/auditd: teach kv about quoted spaces #34069

merged 1 commit into from
Jan 18, 2023

Conversation

efd6
Copy link
Contributor

@efd6 efd6 commented Dec 16, 2022 •
edited
Loading

What does this PR do?

This allows parsing auditd messages that inclued quoted spaces

Why is it important?

Currently these messages cannot be processed.

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 — incomplete see note
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

  • Note that running TESTING_FILEBEAT_MODULES=auditd MODULES_PATH=module GENERATE=1 mage -v pythonIntegTest did not generate the expected case for the added test, and yet the test still passed locally. This needs to be fixed before this PR is merged. Take a look at the integrations partner for this change that does have a complete test, auditd: teach kv about quoted spaces integrations#4858.

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

@efd6 efd6 added bug Filebeat Filebeat Team:Security-External Integrations backport-7.17 Automated backport to the 7.17 branch with mergify backport-v8.6.0 Automated backport with mergify labels Dec 16, 2022
@efd6 efd6 self-assigned this Dec 16, 2022
@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 16, 2022
@efd6 efd6 marked this pull request as ready for review December 16, 2022 21:07
@efd6 efd6 requested a review from a team as a code owner December 16, 2022 21:07
@elasticmachine
Copy link
Collaborator

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

@sonarqubecloud
Copy link

Please retry analysis of this Pull-Request directly on SonarCloud.

@elasticmachine
Copy link
Collaborator

elasticmachine commented Dec 16, 2022 •
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: 2023-01-18T00:12:18.186+0000

  • Duration: 71 min 24 sec

Test stats 🧪

Test Results
Failed 0
Passed 7204
Skipped 742
Total 7946

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the 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!)

@mergify
Copy link
Contributor

mergify bot commented Dec 22, 2022

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 22587-auditd upstream/22587-auditd
git merge upstream/main
git push upstream 22587-auditd

kcreddy
kcreddy approved these changes Dec 26, 2022
View reviewed changes
Copy link
Contributor

@kcreddy kcreddy left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏼
Yeah the test needs to be fixed and conflict resolved before merging.

andrewkroh
andrewkroh reviewed Jan 17, 2023
View reviewed changes
filebeat/module/auditd/log/test/audit-rhel7.log Outdated
@@ -2445,3 +2445,4 @@ type=DAEMON_END msg=audit(1481078697.892:7799): auditd normal halt, sending auid
type=CWD msg=audit(1489639811.480:451): cwd="/home/some_user"
type=PATH msg=audit(1489639811.480:451): item=0 name="/etc/ssh/sshd_config" inode=34485109 dev=08:01 mode=0100600 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:etc_t:s0 objtype=NORMAL
type=USER_CMD msg=audit(1489639825.595:452): pid=1325 uid=1000 auid=1000 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='cwd="/home/some_user" cmd=7461696C202D3230202F7661722F6C6F672F61756469742F61756469742E6C6F67 terminal=pts/0 res=success'
type=ANOM_ABEND msg=audit(1605431420.026:123): auid=12345 uid=123 gid=123 ses=123456789 pid=1234 comm="extproc" reason="memory violation" sig=6
Copy link
Member

@andrewkroh andrewkroh Jan 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Only the first 100 events are verified and output to the golden file if I understand test_modules.py correctly. So if you want this to show up in a golden file then I recommend to move it to its own file.

@efd6
filebeat/module/auditd: teach kv about quoted spaces
7f3ce9d 
Audit messages may contain spaces when the value is quoted, so let the
kv processor know how to deal with this case.
@efd6 efd6 merged commit f5054f7 into elastic:main Jan 18, 2023
@mergify mergify bot mentioned this pull request Jan 18, 2023
Merged
mergify bot pushed a commit that referenced this pull request Jan 18, 2023
@efd6 @mergify
filebeat/module/auditd: teach kv about quoted spaces (#34069)
870a068 
Audit messages may contain spaces when the value is quoted, so let the
kv processor know how to deal with this case.

(cherry picked from commit f5054f7)
@mergify mergify bot mentioned this pull request Jan 18, 2023
Merged
mergify bot pushed a commit that referenced this pull request Jan 18, 2023
@efd6 @mergify
filebeat/module/auditd: teach kv about quoted spaces (#34069)
787bc86 
Audit messages may contain spaces when the value is quoted, so let the
kv processor know how to deal with this case.

(cherry picked from commit f5054f7)
efd6 added a commit that referenced this pull request Jan 18, 2023
@mergify @efd6
filebeat/module/auditd: teach kv about quoted spaces (#34069) (#34297)
1710838 
Audit messages may contain spaces when the value is quoted, so let the
kv processor know how to deal with this case.

(cherry picked from commit f5054f7)

Co-authored-by: Dan Kortschak <[email protected]>
efd6 added a commit that referenced this pull request Jan 18, 2023
@mergify @efd6
filebeat/module/auditd: teach kv about quoted spaces (#34069) (#34298)
6400b56 
Audit messages may contain spaces when the value is quoted, so let the
kv processor know how to deal with this case.

(cherry picked from commit f5054f7)

Co-authored-by: Dan Kortschak <[email protected]>
chrisberkhout pushed a commit that referenced this pull request Jun 1, 2023
@efd6 @chrisberkhout
filebeat/module/auditd: teach kv about quoted spaces (#34069)
ded9daf 
Audit messages may contain spaces when the value is quoted, so let the
kv processor know how to deal with this case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewkroh andrewkroh andrewkroh left review comments

@kcreddy kcreddy kcreddy approved these changes

Assignees

@efd6 efd6

Labels
8.7-candidate backport-7.17 Automated backport to the 7.17 branch with mergify backport-v8.6.0 Automated backport with mergify bug Filebeat Filebeat
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Filebeat auditd ingest pipeline fails at kv processor when field value contains whitespaces
4 participants
@efd6 @elasticmachine @andrewkroh @kcreddy