jupyterhub / kubespawner

Kubernetes spawner for JupyterHub
https://jupyterhub-kubespawner.readthedocs.io
BSD 3-Clause "New" or "Revised" License
541 stars 303 forks source link

Use kubernetes secrets for passing sensitive environment variables to pods #398

Open elgalu opened 4 years ago

elgalu commented 4 years ago

Update by @consideRatio

The environment variable should in order to follow common k8s security practices be mounted from a k8s Secret rather than set directly as a name/value pair in the k8s Pod resource manifest.

# like this...
spec:
  containers:
  - name: users-jupyter-server-container
    env:
      - name: JUPYTERHUB_API_TOKEN
        valueFrom:
          secretKeyRef:
            name: user1-jupyterhub-api-token
            key: JUPYTERHUB_API_TOKEN
# instead of liket his
spec:
  containers:
  - name: users-jupyter-server-container
    env:
      - name: JUPYTERHUB_API_TOKEN
        value: secret-stuff

The difference is that if you do kubectl get pod -o yaml jupyter-user1 then you will see the secret-stuff value if we mount it like we do now, but not if we reference a k8s Secret in order to mount the environment variable. But, for a user with ability to do kubectl get secret -o yaml user1-jupyterhub-api-token then it doesn't matter I think.

Original issue text

https://github.com/jupyterhub/kubespawner/blob/892780ae86c6e74172065ab984bf2d059d00c31f/kubespawner/spawner.py#L1498

https://github.com/jupyterhub/jupyterhub/blob/914a3eaba5c83f639b4b5d8873c6e9d93e809f31/jupyterhub/spawner.py#L722

consideRatio commented 3 years ago

This relates a bit to #110. I think creating a k8s secret and mounting it to the user pod creates quite a bit of complexity to the deployment even though I know it is a practice to not expose a secret directly in the Pod resource manifest.

I'd like to understand the security implications a bit better. Is the security difference regarding that user with get permissions only k8s Pod resources but not k8s Secret resources will be able to get this information that is the security concern only, or is there more?

tirumerla commented 3 years ago

Hi @consideRatio - I was also wondering about this today as i found two env. variables JUPYTERHUB_API_TOKEN & JPY_API_TOKEN when i did kubectl get pod -o yaml <singleuser_pod_name>. Here are my concerns with this approach

Would be great if we can use k8s secrets for these variables as opposed to "plain text" env. variables.

Thanks.

yuvipanda commented 3 years ago

This makes sense. The complexity here will be creating & cleaning up the secret objects with pod start / stop. We could explore doing this with kubernetes garbage collection, although since the secret would need to be created before the pod - not sure if that'll work for that.

We already create PVCs before creating pods, I think we can extend that logic to secrets. While we're there, we could possibly just put all env vars there instead of just 'sensitive' ones.

Happy to review any PRs :)

minrk commented 3 years ago

I think tackling this will give us a chance to generally improve logic around "objects created to go with the pod". Using consistent labels for any object created by kubespawner associated with a user pod should make it manageable to create and clean up everything without lifecycle concerns when e.g. gc is unavailable due to ordering.

tirumerla commented 3 years ago

Thanks @yuvipanda and @minrk for the response. This is currently a security concern for us, and would love to help. Unfortunately I'm not sure where to start, however I'll try helping as much as I can.

yuvipanda commented 3 years ago

https://github.com/jupyterhub/kubespawner/blob/master/kubespawner/spawner.py#L1898 is probably the place to look. We are creating a PVC before starting the pod if needed, so we can attach the PVC to the pod. Same way, we could create a secret and attach it to the pod. https://github.com/jupyterhub/kubespawner/blob/master/kubespawner/spawner.py#L1911 fetched the pod manifest, and we could put the secret binding in there.

We pass env vars to the pod spec from around https://github.com/jupyterhub/kubespawner/blob/master/kubespawner/spawner.py#L1453, so we'll need to modify that to bring in all env vars via secrets.

Hope this helps, @tirumerla!

tirumerla commented 3 years ago

Thanks @yuvipanda for the information. I will take a look.

consideRatio commented 3 years ago

@yuvipanda

This makes sense. The complexity here will be creating & cleaning up the secret objects with pod start / stop. We could explore doing this with kubernetes garbage collection, although since the secret would need to be created before the pod - not sure if that'll work for that.

Actually, the secret can be created after, but the pod will be stuck in ContainerCreating state until the secret is available. Another option that may/may not work is to update the ownerRef of the Secret resource after having created the Pod, but I'm not sure that's allowed to be updated by k8s api-server or suitable from a robustness perspective.

tirumerla commented 3 years ago

Hi @yuvipanda - Apologies for the delay. I took a stab at it, let me know what you think about it.