Chia-Network / chia-operator

A Kubernetes operator for Chia
Apache License 2.0
2 stars 0 forks source link

Running as root is bad #114

Closed skandragon closed 2 days ago

skandragon commented 1 month ago

Should run as non-root

The default is to run as root, which feels like a bad idea if it's not needed.

At the very least, I would like to be able to apply this securityContext to every component:

securityContext:
  runAsUser: 1000
  runAsGroup: 1000
  fsGroup: 1000
  fsGroupChangePolicy: "OnRootMismatch"
  seccompProfile:
    type: "RuntimeDefault"
  capabilities:
    drop:
      - ALL

Not all the components support this, at least not in its entirety. The farmer CRD for instance does not handle feGroup or fsGroupChangePolicy, so switching from an already installed root-running system to a non-root system is difficult.

Locking breaks without root

When running as non-root, the locks are written to /: PermissionError: [Errno 13] Permission denied: '/.chia_keys'

I'm not sure if there are other issues beyond this, as that's as far as I've gotten.

Starttoaster commented 1 month ago

This Issue probably should have been made under the chia-docker repository, since the fact this runs as root is mostly an artifact of the chia-docker image being built as root. There are also features of the docker container (and this operator) that require being run as root, such is the case for timelords because the entrypoint script needs to install some additional utilities to build chiavdf to actually run a timelord (and I believe seeders as well since it binds to a privileged port.)

Though you'd be correct that ideally other components run as non-root. The fact that they run as root is mostly an artifact of the chia-docker container currently running as root, and switching it to non-root being a breaking change to all existing users.

Talks of making a v2 of the image have been had, where the assumption is that to migrate to the v2 of the image you'd maybe need to do some manual effort to move your persistent volume data.

Starttoaster commented 1 month ago

The farmer CRD for instance does not handle feGroup or fsGroupChangePolicy, so switching from an already installed root-running system to a non-root system is difficult.

Can you expound on this? The CRD supports everything the security context API supports, because it is using the same data structure that kubernetes uses for security contexts under the hood.

skandragon commented 1 month ago

The farmer CRD does not have the fsGroup nor fsGroupChangePolicy elements.

For the Farmer CRD, the generated CRD only contains fsGroup and fsGroupChangePolicy in one place, and that is under the initContainer object:

https://github.com/Chia-Network/chia-operator/blob/9c6ce9711d6f20debe77fa5929f9b13fe7937311/config/crd/bases/k8s.chia.net_chiafarmers.yaml#L2497

It seems like this option is missing from all of the container security definitions, but present in all the initContainer specs included.

For the Farmer spec, I would expect it too be under this object: https://github.com/Chia-Network/chia-operator/blob/9c6ce9711d6f20debe77fa5929f9b13fe7937311/config/crd/bases/k8s.chia.net_chiafarmers.yaml#L582

skandragon commented 1 month ago

Though you'd be correct that ideally other components run as non-root. The fact that they run as root is mostly an artifact of the chia-docker container currently running as root, and switching it to non-root being a breaking change to all existing users.

Considering this is money, and people are themselves not computer security experts, this decision to not break installations over what can be a very large security issue seems ill-made. However, there may be ways to run most containers without root given that the following is true:

Starttoaster commented 1 month ago

The farmer CRD does not have the fsGroup nor fsGroupChangePolicy elements.

For the Farmer CRD, the generated CRD only contains fsGroup and fsGroupChangePolicy in one place, and that is under the initContainer object:

https://github.com/Chia-Network/chia-operator/blob/9c6ce9711d6f20debe77fa5929f9b13fe7937311/config/crd/bases/k8s.chia.net_chiafarmers.yaml#L2497

It seems like this option is missing from all of the container security definitions, but present in all the initContainer specs included.

For the Farmer spec, I would expect it too be under this object:

https://github.com/Chia-Network/chia-operator/blob/9c6ce9711d6f20debe77fa5929f9b13fe7937311/config/crd/bases/k8s.chia.net_chiafarmers.yaml#L582

That field isn't underneath initContainer, it's underneath podSecurityContext because fsGroup is a part of the Pod's security context API, not the container security context API. That's a bit confusing because kubernetes named them the same thing for both contexts.

Starttoaster commented 1 month ago

Though you'd be correct that ideally other components run as non-root. The fact that they run as root is mostly an artifact of the chia-docker container currently running as root, and switching it to non-root being a breaking change to all existing users.

Considering this is money, and people are themselves not computer security experts, this decision to not break installations over what can be a very large security issue seems ill-made. However, there may be ways to run most containers without root given that the following is true:

  • Locks can be written to a temporary location, perhaps mapped as a temporary directory
  • Privileged ports are not opened
  • Installation of additional tools can be made by a privileged initContainer that will then allow the main container run as non-root.

I think we're talking past each other a bit here and I'm sorry for that. To be clear though, that talking point was already discussed under the appropriate repository here https://github.com/Chia-Network/chia-docker/issues/163

A v2 of the image that runs as non-root is being discussed but is not being officially planned as of yet. Though your concern and appetite for more security is noted, appreciated, and I want you to know I overall agree with it. It's not currently planned for this operator to support running chia-docker as non-root until chia-docker itself is non-root. You are of course welcome to substitute with your own container image if you decide to build one. By setting:

spec:
  chia:
    image: your-custom-image:tag

In your operator CRDs. chia-docker runs an entrypoint script on start that you'll want to closely follow as the operator leverages that entrypoint script for setting options in your chia configuration.

skandragon commented 1 month ago

That field isn't underneath initContainer, it's underneath podSecurityContext because fsGroup is a part of the Pod's security context API, not the container security context API. That's a bit confusing because kubernetes named them the same thing for both contexts.

I get this:

chia(main): kustomize build | kubectl apply -n chia -f -
secret/chiakey configured
persistentvolumeclaim/chiaroot-farmer-data unchanged
persistentvolumeclaim/chiaroot-wallet-data unchanged
persistentvolumeclaim/plot1 unchanged
persistentvolumeclaim/plot2 unchanged
chiaca.k8s.chia.net/mainnet-ca unchanged
chiaharvester.k8s.chia.net/mainnet unchanged
chianode.k8s.chia.net/mainnet unchanged
chiawallet.k8s.chia.net/mainnet unchanged
The request is invalid: patch: Invalid value: "{\"apiVersion\":\"k8s.chia.net/v1\",\"kind\":\"ChiaFarmer\",\"metadata\":{\"annotations\":{\"kubectl.kubernetes.io/last-applied-configuration\":\"{\\\"apiVersion\\\":\\\"k8s.chia.net/v1\\\",\\\"kind\\\":\\\"ChiaFarmer\\\",\\\"metadata\\\":{\\\"annotations\\\":{},\\\"name\\\":\\\"mainnet\\\",\\\"namespace\\\":\\\"chia\\\"},\\\"spec\\\":{\\\"chia\\\":{\\\"caSecretName\\\":\\\"mainnet-ca\\\",\\\"daemonService\\\":{\\\"type\\\":\\\"LoadBalancer\\\"},\\\"fullNodePeer\\\":\\\"mainnet-node.chia.svc.cluster.local:8444\\\",\\\"rpcService\\\":{\\\"type\\\":\\\"LoadBalancer\\\"},\\\"secretKey\\\":{\\\"key\\\":\\\"key.txt\\\",\\\"name\\\":\\\"chiakey\\\"},\\\"securityContext\\\":{\\\"capabilities\\\":{\\\"drop\\\":[\\\"ALL\\\"]},\\\"fsGroup\\\":1000,\\\"fsGroupChangePolicy\\\":\\\"OnRootMismatch\\\",\\\"runAsGroup\\\":1000,\\\"runAsUser\\\":1000,\\\"seccompProfile\\\":{\\\"type\\\":\\\"RuntimeDefault\\\"}},\\\"timezone\\\":\\\"UTC\\\"},\\\"storage\\\":{\\\"chiaRoot\\\":{\\\"persistentVolumeClaim\\\":{\\\"claimName\\\":\\\"chiaroot-farmer-data\\\"}}}}}\\n\"},\"creationTimestamp\":\"2024-07-23T18:11:57Z\",\"generation\":8,\"managedFields\":[{\"apiVersion\":\"k8s.chia.net/v1\",\"fieldsType\":\"FieldsV1\",\"fieldsV1\":{\"f:status\":{\".\":{},\"f:ready\":{}}},\"manager\":\"manager\",\"operation\":\"Update\",\"subresource\":\"status\",\"time\":\"2024-07-23T18:11:57Z\"},{\"apiVersion\":\"k8s.chia.net/v1\",\"fieldsType\":\"FieldsV1\",\"fieldsV1\":{\"f:metadata\":{\"f:annotations\":{\".\":{},\"f:kubectl.kubernetes.io/last-applied-configuration\":{}}},\"f:spec\":{\".\":{},\"f:chia\":{\".\":{},\"f:caSecretName\":{},\"f:daemonService\":{\".\":{},\"f:type\":{}},\"f:fullNodePeer\":{},\"f:image\":{},\"f:rpcService\":{\".\":{},\"f:type\":{}},\"f:secretKey\":{\".\":{},\"f:key\":{},\"f:name\":{}},\"f:securityContext\":{\".\":{},\"f:capabilities\":{\".\":{},\"f:drop\":{}},\"f:runAsGroup\":{},\"f:runAsUser\":{},\"f:seccompProfile\":{\".\":{},\"f:type\":{}}},\"f:timezone\":{}},\"f:imagePullPolicy\":{},\"f:storage\":{\".\":{},\"f:chiaRoot\":{\".\":{},\"f:persistentVolumeClaim\":{\".\":{},\"f:claimName\":{}}}}}},\"manager\":\"kubectl-client-side-apply\",\"operation\":\"Update\",\"time\":\"2024-08-01T03:35:00Z\"}],\"name\":\"mainnet\",\"namespace\":\"chia\",\"resourceVersion\":\"240106343\",\"uid\":\"153f3df4-a437-49a8-9c59-c939f23ccd90\"},\"spec\":{\"chia\":{\"caSecretName\":\"mainnet-ca\",\"daemonService\":{\"type\":\"LoadBalancer\"},\"fullNodePeer\":\"mainnet-node.chia.svc.cluster.local:8444\",\"image\":\"ghcr.io/chia-network/chia:latest\",\"rpcService\":{\"type\":\"LoadBalancer\"},\"secretKey\":{\"key\":\"key.txt\",\"name\":\"chiakey\"},\"securityContext\":{\"capabilities\":{\"drop\":[\"ALL\"]},\"fsGroup\":1000,\"fsGroupChangePolicy\":\"OnRootMismatch\",\"runAsGroup\":1000,\"runAsUser\":1000,\"seccompProfile\":{\"type\":\"RuntimeDefault\"}},\"timezone\":\"UTC\"},\"imagePullPolicy\":\"Always\",\"storage\":{\"chiaRoot\":{\"persistentVolumeClaim\":{\"claimName\":\"chiaroot-farmer-data\"}}}},\"status\":{\"ready\":true}}": strict decoding error: unknown field "spec.chia.securityContext.fsGroup", unknown field "spec.chia.securityContext.fsGroupChangePolicy"

When I try to apply this:

apiVersion: k8s.chia.net/v1
kind: ChiaFarmer
metadata:
  name: mainnet
spec:
  chia:
    caSecretName: mainnet-ca
    timezone: "UTC"
    daemonService:
      type: LoadBalancer
    rpcService:
      type: LoadBalancer
    fullNodePeer: "mainnet-node.chia.svc.cluster.local:8444"
    secretKey:
      name: "chiakey"
      key: "key.txt"
    securityContext:
      runAsUser: 1000
      runAsGroup: 1000
      fsGroup: 1000
      fsGroupChangePolicy: "OnRootMismatch"
      seccompProfile:
        type: "RuntimeDefault"
      capabilities:
        drop:
          - ALL
  storage:
    chiaRoot:
      persistentVolumeClaim:
        claimName: "chiaroot-farmer-data"

Perhaps I'm confused, but I think securityContext is the proper place to put this in the ChiaFarmer CRD.

Removing the fsGroup* item makes it "work" but since the volumes were previously created with root UID/GID, it cannot mount them.

Starttoaster commented 1 month ago

Sorry, I should have given an example. I mentioned earlier that there are two types of security contexts in Kubernetes. There's the container's security context, and the Pod's security context. fsGroup and fsGroupChangePolicy are part of the Pod's security context field, whereas you are trying to place them underneath the container's security context field. Those fsGroup fields are supported by ChiaFarmer resources, or at least very well should be. But for them to work, you'd need to put them underneath the correct security context, like so:

apiVersion: k8s.chia.net/v1
kind: ChiaFarmer
metadata:
  name: mainnet
spec:
  chia:
    caSecretName: mainnet-ca
    timezone: "UTC"
    daemonService:
      type: LoadBalancer
    rpcService:
      type: LoadBalancer
    fullNodePeer: "mainnet-node.chia.svc.cluster.local:8444"
    secretKey:
      name: "chiakey"
      key: "key.txt"
    securityContext:
      runAsUser: 1000
      runAsGroup: 1000
      seccompProfile:
        type: "RuntimeDefault"
      capabilities:
        drop:
          - ALL
  podSecurityContext:
    fsGroup: 1000
    fsGroupChangePolicy: "OnRootMismatch"
  storage:
    chiaRoot:
      persistentVolumeClaim:
        claimName: "chiaroot-farmer-data"
Starttoaster commented 1 month ago

The Kubernetes documentation corroborates this here: https://kubernetes.io/docs/tasks/configure-pod-container/security-context/

Pointing out the location of the fsGroup setting underneath the Pod's .spec.securityContext instead of .spec.containers[].securityContext.

You can generally think of everything in the ChiaFarmer resource's .spec.chia as config specific to the Chia container within the Pod, or at least directly related to things running in the Chia container (like the k8s Service configuration for exposing Chia ports.) And anything directly under .spec is more bigger picture config like for the whole Pod/Statefulset/Deployment/PVC/etc. I don't know that it's really worth documenting that design philosophy anywhere though, as eventually you add enough thought bubbles to documentation that it becomes unreadable to the average person :) Though I'm making a note to add the different security context fields to the documentation. It makes sense to add, it just seemed like a configuration field that the average user of chia-operator might not use :smile:

skandragon commented 1 month ago

OK, now I get it, and I knew that about k8s. I don't know why took so long to figure it out.

That said, of course, it still cannot work because the container can't run as non-root, but that's another issue entirely.

Starttoaster commented 1 month ago

Glad to help! I'll add docs for security contexts this week. The non-root thing is something I'm tossing up more internally now. I probably can't give a timeline for when to expect it, but I'd like us to get there. It will probably be under a different image name just to give the expectation that there may be some manual effort involved to migrate to it from the current root image. And of course there probably will also continue to be a sudo or root image for at least seeder, but probably also timelord too.

Starttoaster commented 4 weeks ago

Documentation surrounding usage of container/Pod security contexts has been added: https://github.com/Chia-Network/chia-operator/blob/main/docs/all.md#pod-security-contexts

Thanks for diligently pointing out holes in the documentation!

Starttoaster commented 2 days ago

This Issue has been about a few different things at this point, and the documentation has been updated with your excellent suggestions (thank you!) The title for this Issue regarding running as root is inherited by chia-docker. Noting this, I'm going to close the Issue here, but if you have more to bring to the conversation regarding running as root, please feel free to open an Issue on the chia-docker repository to discuss it. This operator will be updated to run as non-root when chia-docker is, which is being considered but has no current ETA at this time, considering the challenges involved with some usages of chia-docker which actually require root, or at least sudo, as well as with maintaining backwards compatibility. It's likely that an iteration of chia-docker that runs as non-root will be under a new image name (potentially designated as a "v2".)