-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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 Processor to enrich auditd events with session view information #37640
Conversation
This processor will enrich process events with additional infomation needed to enable session view in Kibana. This processor can be run on Linux systems, and will use eBPF to enrich auditd events for process exec and exit events. The additional fields that will be added are information on process parent, session leader and process group leader.
Calculate and append entry leader information to enriched processes.
💔 Build Failed
Expand to view the summary
Build stats
Pipeline error
❕ Flaky test reportNo test was executed to be analysed. 🤖 GitHub commentsExpand to view the GitHub comments
To re-run your PR in the CI, just comment with:
|
x-pack/auditbeat/processors/add_session_metadata/add_session_metadata.go
Outdated
Show resolved
Hide resolved
x-pack/auditbeat/processors/add_session_metadata/add_session_metadata.go
Outdated
Show resolved
Hide resolved
x-pack/auditbeat/processors/add_session_metadata/add_session_metadata.go
Outdated
Show resolved
Hide resolved
x-pack/auditbeat/processors/add_session_metadata/add_session_metadata_test.go
Outdated
Show resolved
Hide resolved
pid = uint32(nr) | ||
case uint32: | ||
pid = v | ||
case int, int8, int16, int32, int64: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious how likely are we to get these types (and below).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, this should only be int. In the future, this processor might be used with other beats, so I tried to prepare by accepting all types for pid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think here generics would be helpful to eliminate the usage of reflect
x-pack/auditbeat/processors/add_session_metadata/pkg/processdb/simple_test.go
Outdated
Show resolved
Hide resolved
x-pack/auditbeat/processors/add_session_metadata/pkg/procfs/mock.go
Outdated
Show resolved
Hide resolved
x-pack/auditbeat/processors/add_session_metadata/pkg/procfs/procfs.go
Outdated
Show resolved
Hide resolved
x-pack/auditbeat/processors/add_session_metadata/pkg/procfs/procfs.go
Outdated
Show resolved
Hide resolved
x-pack/auditbeat/processors/add_session_metadata/pkg/procfs/procfs.go
Outdated
Show resolved
Hide resolved
This pull request is now in conflicts. Could you fix it? 🙏
|
Co-authored-by: Dan Kortschak <[email protected]>
❕ Build Aborted
Expand to view the summary
Build stats
Test stats 🧪
Steps errorsExpand to view the steps failures
|
❕ Build Aborted
Expand to view the summary
Build stats
Test stats 🧪
Steps errorsExpand to view the steps failures
|
Remove the DB interface, as there will only be one implementation for it
c9cf706
to
bf38e89
Compare
❕ Build Aborted
Expand to view the summary
Build stats
Test stats 🧪
Test errorsExpand to view the tests failures> Show only the first 10 test failures
|
x-pack/auditbeat/processors/add_session_metadata/processdb/db.go
Outdated
Show resolved
Hide resolved
func ReduceTimestampPrecision(timeNs uint64) uint64 { | ||
return timeNs - (timeNs % (uint64(time.Second.Nanoseconds()) / ticksPerSecond)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this not be expressed with time.Duration(timeNs).Truncate(time.Second/time.Duration(ticksPerSecond))
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it could. I've made this change and changed all related code to work with time.Duration
x-pack/auditbeat/processors/add_session_metadata/provider/ebpf_provider/ebpf_provider.go
Outdated
Show resolved
Hide resolved
Thanks for the good suggestions @andrewkroh. I've made some of the changes, and I'll continue working on the rest |
@mjwolf what are the news here? |
@pierrehilbert Mike is on PTO; he will be back at his desk next week. |
Thx @norrietaylor, no urgency here, just want to ensure we don't have forgotten work in progress :-) |
Remove possibe panics in program initialization, and handle unexpected events more gracefully.
💚 Build Succeeded
cc @mjwolf |
💔 Build Failed
Failed CI StepsHistory
cc @mjwolf |
💔 Build Failed
Failed CI StepsHistory
cc @mjwolf |
💚 Build Succeeded
History
cc @mjwolf |
💚 Build Succeeded
cc @mjwolf |
💔 Build Failed
Failed CI StepsHistorycc @mjwolf |
💚 Build Succeeded
History
cc @mjwolf |
💚 Build Succeeded
History
cc @mjwolf |
💚 Build Succeeded
cc @mjwolf |
💚 Build Succeeded
History
cc @mjwolf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks very good. I just have few minor comments.
return nil, fmt.Errorf("pid %v not found in db: %w", pid, err) | ||
} | ||
|
||
result := ev.Clone() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very expensive operation to perform for each event. The semantics of Beat processors are not well-defined in the interface. Some of the processor clone in order create a transaction-like behavior in order to be able to undo partial changes in case of any failure.
At a minimum, the Clone operation should be performed immediately before the point at which it is necessary (so right before the first action that mutates the event (MergeFieldsDeep
)).
An alternative approach would be to perform some pre-check on the destination map to see if the merge operation can be done safely. Then there would be no need to undo a partial merge operation so we could entirely avoid the clone. We don't need do this now, but we should plan to memory profile this before making it GA.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've moved the Clone to just before MergeFieldsDeep.
But I think it might be possible to remove this completely. It looks like MergeFieldsDeep
won't return error when underRoot
is true (other than the first precondition check), although there's a comment saying the opposite: https://github.com/elastic/elastic-agent-libs/blob/v0.8.0/mapstr/mapstr.go#L375-L380.
Do you know of any conditions where it could return error and leave the event in an inconsistant state?
But I'll leave the clone in for now, and reinvestigate if it's needed when performance testing is done before GA
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it can fail when a field exists in the map that is not an object. Like if the event had a process: 123
field and you wanted to merge in {process: {pid: 123}}
.
* Move event to after some conditions are checked * Use sync.OnceValues in time utils * Add issue reference in comment
…tion (elastic#37640) This adds a add_session_metadata processor to auditbeat which will enrich auditd process events with information needed to enable the Kibana session viewer on the events. In this implementation, eBPF is used to collect information on all processes running on the system, which are added to a process database. When a process event is run through the processor, the DB will be read to retrieve information on the processes related to the process in the event (the processes's parent, session leader,process group leader and entry leader will be retrieved). Then the event will be enriched with the metadata on these related processes, which can enable use of session view on the data
Proposed commit message
This adds a
add_session_metadata
processor to auditbeat which will enrich auditd process events with information needed to enable the Kibana session viewer on the events.In this implementation, eBPF is used to collect information on all processes running on the system, which are added to a process database. When a process event is run through the processor, the DB will be read to retrieve information on the processes related to the process in the event (the processes's parent, session leader,process group leader and entry leader will be retrieved). Then the event will be enriched with the metadata on these related processes, which can enable use of session view on the data
Checklist
- [ ] I have made corresponding changes to the documentationDocumentation is being worked on separately- [ ] I have made corresponding change to the default configuration filesCHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Author's Checklist
How to test this PR locally
By using the below auditd rules and adding the processor configuration, a local auditbeat instance, on a supported Linux system will send ECS events that will work with Kibana session viewer. So it can be tested end-to-end by using this configuration, working interactively with a console session and viewing the session in Kibana.
Audit Rules:
/etc/auditbeat/auditbeat.yml processor config
Related issues
Screenshots
Example of Session View with data provided by Auditbeat and this processor