Skip to content

[Auditbeat] Use a separate netlink socket for control to avoid data congestion.#41207

Merged
nicholasberlin merged 11 commits intomainfrom
nberlin/use_separate_socket_for_control
Oct 16, 2024
Merged

[Auditbeat] Use a separate netlink socket for control to avoid data congestion.#41207
nicholasberlin merged 11 commits intomainfrom
nberlin/use_separate_socket_for_control

Conversation

@nicholasberlin
Copy link
Contributor

@nicholasberlin nicholasberlin commented Oct 11, 2024 •
edited
Loading

Proposed commit message

Use a separate socket for GetStatus.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

Run this program while starting Auditbeat. In a separate terminal run while true; do ss -f netlink | grep auditbeat; done

You will see the auditbeat netlink sockets disappear but Auditbeat will continue to run.
Data will stop flowing to Elasticsearch, and an error message will be pushed Elasticsearch, which will be similar to this:
image

With this PR's patch, the netlink sockets will remain, data will flow to Elasticsearch, and no error message will be pushed.
@nicholasberlin nicholasberlin requested a review from a team as a code owner October 11, 2024 16:27
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Oct 11, 2024
@mergify
Copy link
Contributor

mergify bot commented Oct 11, 2024

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @nicholasberlin? 🙏.
For such, you'll need to label your PR with:

  • The upcoming major version of the Elastic Stack
  • The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit
@mergify
Copy link
Contributor

mergify bot commented Oct 11, 2024

backport-8.x has been added to help with the transition to the new branch 8.x.
If you don't need it please use backport-skip label and remove the backport-8.x label.
@mergify mergify bot added the backport-8.x Automated backport to the 8.x branch with mergify label Oct 11, 2024
fearful-symmetry
fearful-symmetry reviewed Oct 11, 2024
View reviewed changes
Copy link
Contributor

@fearful-symmetry fearful-symmetry left a comment

Choose a reason for hiding this comment

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

just some small nits
auditbeat/module/auditd/audit_linux.go Outdated
client, err := libaudit.NewAuditClient(nil)
defer func() {
if client != nil {
client.Close()
Copy link
Contributor

@fearful-symmetry fearful-symmetry Oct 11, 2024

Choose a reason for hiding this comment

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

I assume that if we move the defer statement to after the error check, we don't need the nil check, and we can just do defer client.close()
Copy link
Contributor Author

@nicholasberlin nicholasberlin Oct 11, 2024

Choose a reason for hiding this comment

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

Ah, makes sense. Thanks.
Copy link
Contributor Author

@nicholasberlin nicholasberlin Oct 11, 2024

Choose a reason for hiding this comment

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

a6b5df3
auditbeat/module/auditd/audit_linux.go Outdated
}()

if err != nil {
return nil, err
Copy link
Contributor

@fearful-symmetry fearful-symmetry Oct 11, 2024

Choose a reason for hiding this comment

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

nit: can we get a fmt.Errorf() here?
Copy link
Contributor Author

@nicholasberlin nicholasberlin Oct 11, 2024

Choose a reason for hiding this comment

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

a6b5df3
@andrewkroh andrewkroh added Auditbeat Team:Security-Linux Platform Linux Platform Team in Security Solution labels Oct 14, 2024
@elasticmachine
Copy link
Contributor

elasticmachine commented Oct 14, 2024

Pinging @elastic/sec-linux-platform (Team:Security-Linux Platform)
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Oct 14, 2024
@nicholasberlin
Copy link
Contributor Author

nicholasberlin commented Oct 14, 2024

@fearful-symmetry I re-worked the whole thing, please have another look. Thanks.
@fearful-symmetry
Copy link
Contributor

fearful-symmetry commented Oct 14, 2024

@nicholasberlin looks like mostly linter errors from old code, we need to use value, ok := interface.(type) when you're typecasting.
@nicholasberlin
Copy link
Contributor Author

nicholasberlin commented Oct 14, 2024

we need to use value, ok := interface.(type)

Thanks! Will get into it tomorrow.
@nicholasberlin
Copy link
Contributor Author

nicholasberlin commented Oct 15, 2024

@fearful-symmetry ready for review now. Thanks again.
andrewkroh
andrewkroh reviewed Oct 16, 2024
View reviewed changes
@nicholasberlin nicholasberlin merged commit ac81fbd into main Oct 16, 2024
@nicholasberlin nicholasberlin deleted the nberlin/use_separate_socket_for_control branch October 16, 2024 18:17
mergify bot pushed a commit that referenced this pull request Oct 16, 2024
@nicholasberlin @mergify
[Auditbeat] Use a separate netlink socket for control to avoid data c…
872a91b 
…ongestion. (#41207)

(cherry picked from commit ac81fbd)
@mergify mergify bot mentioned this pull request Oct 16, 2024
Merged
3 tasks
nicholasberlin added a commit that referenced this pull request Oct 16, 2024
@mergify @nicholasberlin
[Auditbeat] Use a separate netlink socket for control to avoid data c…
27a6078 
…ongestion. (#41207) (#41262)

(cherry picked from commit ac81fbd)

Co-authored-by: Nicholas Berlin <56366649+nicholasberlin@users.noreply.github.com>
belimawr pushed a commit to belimawr/beats that referenced this pull request Oct 18, 2024
@nicholasberlin @belimawr
[Auditbeat] Use a separate netlink socket for control to avoid data c…
e6e3f14 
…ongestion. (elastic#41207)
@nicholasberlin nicholasberlin added the backport-8.16 Automated backport with mergify label Oct 24, 2024
@hakong
Copy link

hakong commented Oct 31, 2024

FYI: Searching for this error 'failed to get audit status before adding rules: failed to get audit status ack: error receiving audit reply: no buffer space available' didn't give me any results on Google or Github. The reason being the error message is in a screenshot, not in plaintext.

image
image

I ended up creating a ticket with Elastic and after a few days of messages I was finally told it would be fixed in 8.16. I then had to dig through the MR's for the 8.16 branch to find anything auditd related. That led me to this PR.

Would be good to put the error message in plaintext in the issue/PR/MR so people can actually search for it :)
@nicholasberlin nicholasberlin mentioned this pull request Dec 13, 2024
Open
@khushijain21 khushijain21 mentioned this pull request Jun 23, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Auditbeat backport-8.x Automated backport to the 8.x branch with mergify backport-8.16 Automated backport with mergify bugfix Team:Security-Linux Platform Linux Platform Team in Security Solution

5 participants

@nicholasberlin @elasticmachine @fearful-symmetry @hakong @andrewkroh