search

SAP-archive / karydia

Kubernetes Security Walnut
Other
77 stars 10 forks source link

Add default for automountServiceAccountToken feature #55

Closed marwinski closed 5 years ago

marwinski commented 5 years ago

The automountServiceAccountToken feature is too prohibitive at the moment as it prevents commands like kubectl run to work.

We therefore need an additional more permissive mode that just set the property to false if it not is not set in the pod spec.

I'd propose to name the property setDefaultValueFalse (see https://github.com/karydia/karydia/blob/master/manifests/example-karydia-security-policy.yml)

schu commented 5 years ago

Note that we don't enable karydiasecuritypolicy admission (--enable-ksp-admission) in current master, as we were not sure to continue with the KSP approach at all, since it turned out to be rather complicated to configure right (as PSPs).

But we added a similar feature to the karydia admission handler, https://github.com/karydia/karydia/blob/76a5e5ee55ebf35a42c17fb45979b1094f0638d7/pkg/admission/karydia/karydia.go#L107-L118 (there configured through namespace annotations).

We therefore need an additional more permissive mode that just set the property to false if it not is not set in the pod spec.

We had tested this before and it doesn't seem to be possible like that, unfortunately. While we can mutate automountServiceAccountToken to false in the mutating webhook, it doesn't have the intended effect: the service token is mounted nonetheless.

This is due to how the Kubernetes ServiceAccount admission handler works (it's also a mutating admission controller as karydia is): https://github.com/kubernetes/kubernetes/blob/master/plugin/pkg/admission/serviceaccount/admission.go#L151-L190

When the ServiceAccount admission handler gets an admission request for a pod, it mounts the token unless

As far as I understand at the moment, we need to find a different path forward.

schu commented 5 years ago

We therefore need an additional more permissive mode that just set the property to false if it not is not set in the pod spec.

We had tested this before and it doesn't seem to be possible like that, unfortunately. While we can mutate automountServiceAccountToken to false in the mutating webhook, it doesn't have the intended effect: the service token is mounted nonetheless.

There's some work-in-progess code in https://github.com/karydia/karydia/pull/45 that shows the problem, IIRC.

marwinski commented 5 years ago

Thanks, I did not see this. Well this makes the whole item rather pointless and we should probably stick with what we have at the moment.

sure to continue with the KSP approach at all, since it turned out to be rather complicated to configure right (as PSPs).

What is wrong with that? I must say I quite liked that approach :-) Sure we can turn this all on by default and provide an option for the user to ignore / turn off.

dacappo commented 5 years ago

We had tested this before and it doesn't seem to be possible like that, unfortunately. While we can mutate automountServiceAccountToken to false in the mutating webhook, it doesn't have the intended effect: the service token is mounted nonetheless.

Just took a look at the k8s code. As far as I understood, the ServiceAccount admission handler configures the volumes before our mutating webhook has its turn. The volumes are configured, but not actually mounted at this point.

What I would propose is to delete the volume of the ServiceAccountToken when automountServiceAccountToken is not defined. Additionally automountServiceAccountToken should be set to false - albeit this would be only for clarification when looking up the YAML afterward.