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

Drop all capabilities by default in Elastic Agent containers #1794

Merged
merged 4 commits into from
Apr 25, 2024

Conversation

mrodm
Copy link
Contributor

@mrodm mrodm commented Apr 22, 2024

Part of #787

In docker/docker-compose there are some capabilities that are added by default to every container (https://docs.docker.com/engine/reference/run/#runtime-privilege-and-linux-capabilities).
This PR ensures that agents are started with the minimum required capabilities, dropping by default all of them.

If ALL it is defined in cap_drop field, then it just adds the Linux capabilities for the container defined in cap_add:
https://github.com/moby/moby/blob/82d8f8d6e6dbc88ed437b8bf6c38399b46ba7d93/oci/caps/utils.go#L112-L114

@mrodm mrodm self-assigned this Apr 22, 2024
@@ -18,6 +18,8 @@ services:
- {{ . }}
{{- end }}
{{ end }}
cap_drop:
- ALL
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about trying to drop all capabilities also in the main docker compose, and test with integrations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll update those scenarios and run a test with integrations.

Wondering what to do in the template used for custom agents (servicedeployer). For that case, packages could also define some cap_drop. It's also true that until now some capabilities are just added with cap_add. But looking at the code of moby, if ALL is present in cap_drop , all capabilities are drop...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like there are no issues when cap_drop ALL is set:
elastic/integrations#9694

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we update also custom agent template ? As there are no packages using cap_drop in the integrations repository, probably it's a good idea, WDYT ?

https://github.com/elastic/elastic-package/blob/5e322d7be61a739b855689aa0c4342abd5fa9d68/internal/servicedeployer/_static/docker-custom-agent-base.yml

Copy link
Member

Choose a reason for hiding this comment

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

Yes, good idea.

@mrodm
Copy link
Contributor Author

mrodm commented Apr 24, 2024

test integrations

@elasticmachine
Copy link
Collaborator

Created or updated PR in integrations repository to test this version. Check elastic/integrations#9694

@mrodm mrodm marked this pull request as ready for review April 24, 2024 16:15
@mrodm mrodm requested a review from jsoriano April 24, 2024 16:15
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Nice 👍

@mrodm
Copy link
Contributor Author

mrodm commented Apr 25, 2024

test integrations

@elasticmachine
Copy link
Collaborator

Created or updated PR in integrations repository to test this version. Check elastic/integrations#9694

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @mrodm

@mrodm mrodm changed the title Drop all capabilities by default in independent agents Drop all capabilities by default in Elastic Agent containers Apr 25, 2024
@mrodm mrodm merged commit 9c3881e into elastic:main Apr 25, 2024
3 checks passed
@mrodm mrodm deleted the drop_all_caps_default branch April 25, 2024 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants