feat: use security contexts and mount configs instead of chown init containers #223
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The prior example used apk to add openssl and curl, which needed root. This makes the certs in an initContainer (alpine/openssl), then sends to k8s in a separate container (alpine/curl), sharing state through an emptyDir
…ing user) This relies on the fact that an emptyDir is set up with appropriate permissions for whichever userid is running in the pod. The prior solution required running chown/chmod by root in the init container. This obviates the need for root, which is not permitted in many kubernetes installs as a security precaution. Fixes zitadel#177 and also fixes zitadel#178 (since there is no chown anymore).
hifabienne
added
the
os-contribution
stebenz
previously approved these changes
Jul 18, 2024
This makes files user/group readable but not by others. Without this, mode is set to 0644 (rw for user, r for group and others). Also removes an unused emptyDir in debug replicaset
|
Thanks for looking over this! I agree that it's much cleaner to just use fsGroup and avoid the copying altogether. I think if going with that, though, it's probably safer to explicitly set defaultMode for the involved volumes (at least on the cluster I checked on, without doing so you end up with rw-r--r--). I've pushed an additional commit with that change (and cleaning up an emptyDir volume in the debug replicaSet which is not needed if not copying secrets). |
eliobischof
approved these changes
Jul 19, 2024
There was a problem hiding this comment.
hey @bpow thanks for your help to make the chart more secure 🥳
If you'd like to have a small gift in return, please send us a mail to [email protected]. We will send you a form with questions about your address and shirt size.
eliobischof
changed the title
eliobischof
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This implements several changes to allow the pods to run as arbitrary user id
throughout all of the lifecycle stages. Primarily this is by avoiding the need to
chown for certificates, etc., instead copying them to an emptyDir which gives them
appropriate ownership for the pod's securityContext.
The postgres cert job example also needed modification because the prior version
required root to be able to install additional packages for cert creation. Instead
I used an alpine/openssl image to create the certs in an initContainer, then
alpine/curl to store the certs in relevant
secrets. The middle commit (currently6256125 if it doesn't get rebased) just prevents writing certain potentially-sensitive
information to logs in case those aren't locked down as well as you'd like. I'd be
OK rebasing without that commit if desired.
Relevant "user story" is in #177
I'm not familiar with the testing mechanisms provided by helm, but for "verification",
I have deployed to two different clusters (one kubernetes and one openshift) that do
not permit containers to run as root (optionally defining runAsUser in relevant
securityContext to provide a specific userId rather than the runtime-assigned id).
In both cases, I was able to exec a shell in the zitadel-debug replicaset
container and verify that the files created had ownership of the running userId
and mode of 400.
Definition of Ready