[Host Profiler] Tighten seccomp profile#3042
Conversation
theomagellan
added
enhancement
|
@codex review |
|
🎯 Code Coverage (details) 🔗 Commit SHA: 358d49e | Docs | Datadog PR Page | Give us feedback! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4648de6fae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "read", | ||
| "readlinkat", | ||
| "recvfrom", | ||
| "recvmsg", |
There was a problem hiding this comment.
When the host-profiler exports to the Datadog intake over an endpoint configured as a hostname, the Go resolver reads UDP DNS replies via recvfrom (net.UDPConn.ReadFrom goes through internal/poll.FD.ReadFrom/syscall.Recvfrom). With SCMP_ACT_ERRNO as the default action, removing this syscall causes those lookups to fail under the generated profile, so profile uploads break in the common hostname-endpoint path. Please leave recvfrom in the allowlist unless all configured exporter endpoints are guaranteed to be numeric or agent-local.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
If I understand correctly, this would mean profiles wouldn't be able to be sent, which is not happening here.
Profiles look healthy on all versions and arch.
There was a problem hiding this comment.
I think this depends on how you specified the URL to upload to. What type of URLs did you test ?
There was a problem hiding this comment.
We discussed this and Théo made the relevant tests 👍
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3042 +/- ##
==========================================
+ Coverage 42.24% 42.36% +0.11%
==========================================
Files 337 337
Lines 28951 28984 +33
==========================================
+ Hits 12230 12278 +48
+ Misses 15916 15899 -17
- Partials 805 807 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
667b58a to
358d49e
Compare
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: queued
mergequeue-status: in_progress
and removed
mergequeue-status: queued
labels
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: done
and removed
mergequeue-status: in_progress
labels
4 participants
What does this PR do?
This PR remove 23 syscalls from the original list of allowed syscalls.
Adds syscalls to allow the profiler to rename its log file (as observed in test cluster)
Motivation
These were needed before AllowPrivilegeEscalation was manually set to false.
Additional Notes
Anything else we should know when reviewing?
Minimum Agent Versions
Are there minimum versions of the Datadog Agent and/or Cluster Agent required?
Describe your test plan
Profiles running under new seccomp
Checklist
bug,enhancement,refactoring,documentation,tooling, and/ordependenciesqa/skip-qalabel