-
Notifications
You must be signed in to change notification settings - Fork 138
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
Dangerous file modifications by chown
init container
#485
Comments
The agent only needs access to the checkpoint directory and log files. So, maybe we should modify the permissions of log files only. PR: #486 |
That definitely sounds like a step in the right direction. Do we have any way to confirm that |
Wow, that's great! Sounds like a really good solution. I'm no expert in Linux's ACL, but I'm pretty sure we identified no issues with that line. Thanks a lot for this fix! I will test this out and report back if anything seems broken. |
A question to someone more knowledgeable about |
@lindhe, restore flag can be used to delete ACLs but it requires root access. So, the collector itself cannot do the cleanup. You need to run separate job for that. |
@harshit-splunk perhaps a new pod (with root access) can be started upon uninstall via Helm's |
Hello!
We are Splunk Observability Cloud customers using this chart to collect logs and metrics from our Kubernetes clusters. We have been having issues caused by the
chown
init container.Background
A while back, we noticed some very strange behaviour in some of our pods. Our Ingress Controllers crashed seemingly out of random, and were stuck in
CrashLoopBackOff
. Upon investigating, it turned out it could no longer read the/etc/resolv.conf
, which would obviously be a pretty big issue for a reverse proxy... To our surprise, the file access permissions looked very strange. It didn't haveread
permissions, which it normally would, and it had the sgid bit set! And it also, seemingly out of nowhere, had group ownership set to gid 20000. How peculiar... And it turned out that this affected all pods in our cluster, it just happened to be the Ingress Controllers that showed clear symptoms from this.After a lot of debugging, we realized that the culprit was that we had done a security audit where we decided that we should disallow running containers as root. When implementing that, we changed all of our applications to run as non-root users, including this OpenTelemetry chart. So, that was quite the surprise – lowering Otel's privileges caused harm to our system!
When we understood the problem, we were able to mostly rescue things by reverting our non-root changes in Otel and manually restoring file permissions to more reasonable settings. We think we found reasonable settings, but we are not sure since it's very hard to know what the state of the file system was before the
chown
init container did its thing (especially considering the sticky bits, which makes it impossible to know the intended file permissions for newly created files)!Ideally, we should read through all changes in all charts and all images before we install them in our clusters. That, however, is pretty hard to do in practice. We missed this snippet, partly because we didn't expect so damaging changes when enabling rootless operation.
Problems
There are a multitude of issues with the current design, and we think it is good if we try and list them so we can consider their impacts when implementing a solution. Please help us identify if there are other problems that we have missed in our initial analysis!
Show-stopping problems
/var/lib/docker/containers
does not only hold logs but also other system configuration files (at leasthostname
,hosts
andresolv.conf
), it is unreasonable to change permissions of all files in that directory.Uncomfortable problems
Solutions
Granted this is a hard problem to solve. The pod has to be able to read the logs somehow, and if it's not running as the root user then it has to have some trick up its sleeve to do its job. But we think that the current solution is way too damaging and dangerous to be allowed in production, so we should try and fix this.
One way you could make it more likely to catch these kinds of issues could be to make sure you test on at least one cluster per container runtime, e.g. one Docker based cluster (minikube, RKE, etc.), one containerd based cluster (AKS) and one CRIO based cluster (OpenShift maybe?).
S1
One solution, which is probably sub-optimal, is to only allow running as
root:root
. That's the nature of some applications. ¯\_(ツ)_/¯S2
Another solution might be to enable a clear warning to the user when this option is enabled. We think that this is not a safe enough solution so it should be avoided. But in case you insist having it implemented the current way, then at least it would be a minor improvement with a very clear warning.
S3
A third solution may be to disallow running as
20000:20000
but instead run as20000:root
. That's what we have gone with currently. As far as we can tell, that grants us read permission to all logs while not requiring any system-wide file changes. While we think this seems like the best compromise between security and usability, we are not 100% it always works as intended. It seems to work fine for us (having tested in AKS, RKE and minikube), but it's really hard to know exactly what one gets access to when running as20000:root
on arbitrary clusters. The exact security implications of running with grouproot
are unclear.The text was updated successfully, but these errors were encountered: