helm-charts icon indicating copy to clipboard operation
helm-charts copied to clipboard

[grafana] Add `DAC_READ_SEARCH` to `initChownData` and `readOnlyRootFilesystem`

Open jcpunk opened this issue 7 months ago • 10 comments

This should resolve the regression from https://github.com/grafana/helm-charts/pull/3684

jcpunk avatar May 05 '25 13:05 jcpunk

@andreas-heatly are you able to test this to prevent an outage on your side?

jkroepke avatar May 05 '25 15:05 jkroepke

I tested the restart usecase explained in https://github.com/grafana/helm-charts/issues/3699 and this fixes that at least

  1. Install the chart helm upgrade -i my-release grafana/grafana --set initChownData.enabled=true --set persistence.enabled=true --wait
  2. Once the pod is running, delete it
  3. Observe that it cannot restart because the initcontainer fails with
chown: /var/lib/grafana/csv: Permission denied
chown: /var/lib/grafana/png: Permission denied
chown: /var/lib/grafana/pdf: Permission denied
19:52:16 dell-e7440 [--0] ~  $
  1. updated the deployment with the changes proposed here
  2. new pod comes up

Kimi450 avatar May 05 '25 20:05 Kimi450

Tested this - does not work with psp:

message: 'pods "grafana-64f4f78b5f-xkwr8" is forbidden: violates PodSecurity "baseline:latest":                               
      non-default capabilities (container "init-chown-data" must not include "DAC_READ_SEARCH"                                    
      in securityContext.capabilities.add)'

Don't think we have to give away more than the baseline policy?!

markussiebert avatar May 09 '25 18:05 markussiebert

baseline policy

Generally, chown is a privileged operation. If you want to be compliant with baseline, just disable the initContainer. In modern Kubernetes environment, its generally not recommend to use that approach.

jkroepke avatar May 09 '25 18:05 jkroepke

Breaking compat with baseline is not something I'm interested in.

There is some sort of strange interaction between the init-container and the pvc. I don't think I'll have time to look at this for a while... I'll close this PR out.

Interested folks at least have a hint something is tricky with the explicit readOnlyRootFilesystem: false.

jcpunk avatar May 09 '25 18:05 jcpunk

Breaking compat with baseline is not something I'm interested in.

Sorry to interrupt here, but the chown init container will never be compatible with baseline.

Changing file owner is an operation done by the root account. This is normally done by securityContext.fsGroups. In production deployments, this initContainer not be not used.

But it's still valuable to increase the hardening in that context.

jkroepke avatar May 09 '25 18:05 jkroepke

But I don't understand - shouldn't the default be the hardened? I am only here, because of the issues. Never decided to activate the init container. Never had any psp issues until the cap of this pr was added. And chown alone doesn't violate the psp.

markussiebert avatar May 09 '25 21:05 markussiebert

But how the PSP can affect your deployment, if chmod container is not enabled on your deployment?

jkroepke avatar May 10 '25 06:05 jkroepke

The default is enable chown

markussiebert avatar May 10 '25 07:05 markussiebert

Thats the root issue. this should be not enabled by default.

jkroepke avatar May 10 '25 07:05 jkroepke