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 container securityContext by default #1798

Conversation

consideRatio
Copy link
Member

@consideRatio consideRatio commented Oct 4, 2020

This can help us avoid usage of root by default which can cause clusters with PodSecurityPolicy resources to stop the Helm chart from functioning.

In practice, what I do in this PR is to create Helm chart configuration to set all Pod's containers' securityContext, and let this configuration default to have a securityContext like this be injected to all of our managed containers within the pods.

# some of our k8s Deployment resources
spec:
  template:
    spec:
      containers:
        - image: ...
          # ...
          securityContext:
            runAsUser: 65534
            runAsGroup: 65534
            allowPrivilegeEscalation: false

A securityContext of a pod with a runAsUser directive will force the container to startup as that user instead of the default: root. This can add another layer of security to by letting the containers we run not end up escalating privileges and ending up abusing some k8s vulnerability to do harmful stuff.

Fixes #1386, but not #1408.

singleuser.containerSecurityContext ?

This PR does not add singleuser.containerSecurityContext, yet, because it is quite a bit more complicated at the moment. KubeSpawner expose some configuration that in the end goes into either the pod's securityContext or the container's securityContext. Here is an overview.

KubeSpawner.* PodSecurityContex SecurityContext
fs_gid fsGroup -
supplemental_gids supplementalGroups -
run_as_uid - runAsUser
run_as_gid - runAsGroup
privileged - privileged

Breaking change

  • proxy.containerSecurityContex has been renamed to proxy.chp.containerSecurityContex

I deem that this can be said to close #937 good enough, but only properly by setting singleuser.cloudMetadata.blockWithIptables: false.

Closes #1491, Closes #1677

Copy link
Collaborator

@yuvipanda yuvipanda left a comment

Choose a reason for hiding this comment

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

I like! Disabling privilege escalation is my fav!

jupyterhub/values.yaml Outdated Show resolved Hide resolved
@yuvipanda
Copy link
Collaborator

@consideRatio This LGTM! Happy to merge once the conflict is resolved. <3 for better security by default!

@consideRatio consideRatio force-pushed the pr/set-container-securityContext-by-default branch from 67a83d6 to dc2f060 Compare October 8, 2020 11:56
This can help us avoid usage of root by default which can cause clusters
with PodSecurityPolicy resources to stop the Helm chart from
functioning.
@consideRatio consideRatio force-pushed the pr/set-container-securityContext-by-default branch from dc2f060 to cc6c02d Compare October 8, 2020 12:09
@consideRatio
Copy link
Member Author

@yuvipanda merge conflicts resolved!

@yuvipanda yuvipanda merged commit 825b352 into jupyterhub:master Oct 8, 2020
@yuvipanda
Copy link
Collaborator

Thanks, @consideRatio!

@consideRatio
Copy link
Member Author

Wieee thank you for taking the time to review this @yuvipanda !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants