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

auditbeat/module/file_integrity: clean paths obtained from fsnotify before sending #28354

Merged
merged 1 commit into from
Oct 13, 2021

Conversation

efd6
Copy link
Contributor

@efd6 efd6 commented Oct 11, 2021

What does this PR do?

This cleans incorrectly constructed, but valid, paths obtained from fsnotify and makes relative paths useful.

Note that the code is duplicated across the fsnotify and fsevent functions due to separation by build tags and differences in the types that are being handled. They could be factored into a single func absolutePathFor(path string, logger *logp.Logger) string, but there was no obvious place to put that.

Why is it important?

Currently paths of files identified in root by the file integrity module have path with either a // prefix or with a ./ if a relative or empty path configuration is provided. This change resolves relative paths to absolute event paths and cleans paths with extraneous path separators and dots.

Testing the root path behaviour in automated testing does not seem feasible.

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

N/A

How to test this PR locally

Run auditbeat with the following configuration

- module: file_integrity
  paths:
  - /bin
  - /usr/bin
  - /sbin
  - /usr/sbin
  - /etc
  -

or

- module: file_integrity
  paths:
  - /bin
  - /usr/bin
  - /sbin
  - /usr/sbin
  - /etc
  - /

and then create a file in / or in the current working directory for auditbeat.

Related issues

Use cases

N/A

Screenshots

N/A

Logs

N/A

@efd6 efd6 requested a review from a team as a code owner October 11, 2021 21:56
@elasticmachine
Copy link
Collaborator

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

@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 Oct 11, 2021
@efd6 efd6 requested a review from andrewkroh October 11, 2021 21:56
@efd6 efd6 force-pushed the auditbeat/cleanpaths branch from 4bc1d67 to a3c7ed9 Compare October 11, 2021 21:58
@elasticmachine
Copy link
Collaborator

elasticmachine commented Oct 11, 2021

💚 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-10-13T21:14:37.142+0000

  • Duration: 62 min 11 sec

  • Commit: 7e594c5

Test stats 🧪

Test Results
Failed 0
Passed 902
Skipped 210
Total 1112

💚 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.

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

LGTM. Just one minor change requested for the logger.

auditbeat/module/file_integrity/eventreader_fsevents.go Outdated Show resolved Hide resolved
…nd fsevents before sending

Document that while users should configure with an absolute path, the module will
attempt to resolve paths to their absolute equivalent.
@efd6 efd6 force-pushed the auditbeat/cleanpaths branch from a3c7ed9 to 7e594c5 Compare October 13, 2021 21:14
@efd6 efd6 merged commit 58224e5 into elastic:master Oct 13, 2021
@efd6 efd6 deleted the auditbeat/cleanpaths branch October 13, 2021 22:51
mergify bot pushed a commit that referenced this pull request Oct 13, 2021
…nd fsevents before sending (#28354)

Document that while users should configure with an absolute path, the module will
attempt to resolve paths to their absolute equivalent.

(cherry picked from commit 58224e5)
andrewkroh pushed a commit that referenced this pull request Oct 28, 2021
…nd fsevents before sending (#28354) (#28417)

Document that while users should configure with an absolute path, the module will
attempt to resolve paths to their absolute equivalent.

(cherry picked from commit 58224e5)

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

@efd6 changelog entries should go to the CHANGELOG.next.asciidoc file. CHANGELOG.asciidoc is built during the release process

andresrc added a commit to andresrc/beats that referenced this pull request Oct 30, 2021
Icedroid pushed a commit to Icedroid/beats that referenced this pull request Nov 1, 2021
…nd fsevents before sending (elastic#28354)

Document that while users should configure with an absolute path, the module will
attempt to resolve paths to their absolute equivalent.
@efd6
Copy link
Contributor Author

efd6 commented Nov 1, 2021

@andresrc Thanks. I knew this but fumbled. Sorry.

andresrc added a commit that referenced this pull request Nov 1, 2021
* Fix changelog entry for #28354

* Fix lines that were added
mergify bot pushed a commit that referenced this pull request Nov 1, 2021
* Fix changelog entry for #28354

* Fix lines that were added

(cherry picked from commit 7ec1012)
mergify bot pushed a commit that referenced this pull request Nov 1, 2021
* Fix changelog entry for #28354

* Fix lines that were added

(cherry picked from commit 7ec1012)
andresrc added a commit that referenced this pull request Nov 1, 2021
* Fix changelog entry for #28354

* Fix lines that were added

(cherry picked from commit 7ec1012)

Co-authored-by: Andres Rodriguez <andres.rodriguez@elastic.co>
andresrc added a commit that referenced this pull request Nov 1, 2021
* Fix changelog entry for #28354

* Fix lines that were added

(cherry picked from commit 7ec1012)

Co-authored-by: Andres Rodriguez <andres.rodriguez@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v7.16.0 Automated backport with mergify bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

File_integrety module shows inconsistent convention when the file path is the root.
4 participants