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

Set Default file mode to 0600 #21

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Set Default file mode to 0600 #21

wants to merge 7 commits into from

Conversation

voxel01
Copy link
Member

@voxel01 voxel01 commented Apr 29, 2024

Config files could contain connection strings to databases, exporters,... this could result in (local) privilege escalation if every user is able to read those config files
So in terms of having a more secure base config, I think setting default file mode to 0600 prevents failures.

@voxel01 voxel01 self-assigned this Apr 29, 2024
manifests/init.pp Outdated Show resolved Hide resolved
@bastelfreak
Copy link
Member

@cyberkov thanks for the PR, can you please rebase against our master branch? And is that really a backwards-incompatible change?

@cyberkov
Copy link
Contributor

cyberkov commented May 3, 2024

@cyberkov thanks for the PR, can you please rebase against our master branch? And is that really a backwards-incompatible change?

I think it would be fine. The config file belonged to root before and now to the user otelcol. root would be able to read it anyway :)

@smortex
Copy link
Member

smortex commented May 3, 2024

If we want to be very conservative, this should be considered a backwards-incompatible change for the rest of the applications: if one was picking in otelcol configuration to read something, the new behavior will prevent that.

Another side note: if the service own the configuration file, it can change it permissions and content. That may not be an issue, but it is generally better in case of vulnerability for the configuration to be immutable by the pwned service and be owner by another user (root), belong to a group of the application, and have permission 0640.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants