falcosecurity / charts

Community managed Helm charts for running Falco with Kubernetes
Apache License 2.0
242 stars 286 forks source link

Provide good container securityContext by default for each chart #481

Open jemag opened 1 year ago

jemag commented 1 year ago

Motivation This would improve the default security out of the box for helm chart users. If the containers currently support it, there isn't much downside to improving the default security.

It also helps clarifying to the end-user that these values are officially supported and will not cause any problem with the containers (e.g.: readOnlyRootFilesystem: true could for example cause problems for containers expecting to write to specific directories). By having them already specified, the end-user does not need to do extensive testing to ensure it does not cause any issue.

Feature

Have each Helm chart container securityContext provide the highest default that they can sustain. For example, this would, in general, be a good default in line with Kubernetes PSS (https://kubernetes.io/docs/concepts/security/pod-security-standards/) to try and reach towards (obviously just a target, not possible to attain for falco itself):

securityContext:
  allowPrivilegeEscalation: false
  capabilities:
    drop:
    - ALL
  readOnlyRootFilesystem: true
  runAsNonRoot: true
  runAsUser: 1000
  seccompProfile:
    type: RuntimeDefault

Alternatives Each end-user has to do extensive testing to figure out which securityContext setting is appropriate and which may cause problem. This is also risky since some settings may not cause errors until the container has been running for a while.

Additional context

Falco sidekick does not currently provide any securityContext nor a commented one : https://github.com/falcosecurity/charts/blob/6fff4e1e75c43af742596f478bf86e7723aa08da/falcosidekick/values.yaml#L23-L24 This then leaves the end-user to do extensive testing on his own to figure out the highest/optimal securityContext

Falco exporter does not provide default securityContext but gives some better suggestion through comment:https://github.com/falcosecurity/charts/blob/6fff4e1e75c43af742596f478bf86e7723aa08da/falco-exporter/values.yaml#L73-L80 It would be great to have a good default provided out of the box as suggested in the Feature section

Event-generator seems to be in a similar situation as falco-exporter

Falco itself is very hard to find the optimal securityContext for the end-user. There are currently some default configuration implemented, but they are not all working at the moment (see https://github.com/falcosecurity/falco/issues/2487). It would be nice to have a strong default securityContext here as well, especially since it is not easy to find the appropriate settings in each context (e.g.: ebpf, modern-bpf, module, etc.) In this case, the securityContext would be dynamically computed.

jemag commented 1 year ago

This would cover and close https://github.com/falcosecurity/charts/issues/425 as well

leogr commented 1 year ago

cc @alacuku @therealbobo @Andreagit97

therealbobo commented 1 year ago

I'm already taking a look at this possible enhancement: it's just matter of finding the minimum set of privileges needed (e.g. falco-exporter needs to run as root).

leogr commented 1 year ago

I'm already taking a look at this possible enhancement: it's just matter of finding the minimum set of privileges needed (e.g. falco-exporter needs to run as root).

Actually, not just falco-exporter, but any application needs to connect to the gRPC socket.

jemag commented 1 year ago

Thank you for looking into it.

As a side note, it seems a bit misleading to have these comments in falco-exporter values if it needs to be run as root (instead of nonRoot and user 1000): https://github.com/falcosecurity/charts/blob/6fff4e1e75c43af742596f478bf86e7723aa08da/falco-exporter/values.yaml#L73-L80

therealbobo commented 1 year ago

Totally agree. In a short time I'll open a pr with the comments removed and an actual securityContext written adhoc for each container. 😄

poiana commented 1 year ago

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

jemag commented 1 year ago

/remove-lifecycle stale

R011y commented 1 year ago

Any update here?

leogr commented 1 year ago

/remove-lifecycle stale

cc @alacuku

leogr commented 1 year ago

/help

poiana commented 1 year ago

@leogr: This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-help command.

In response to [this](https://github.com/falcosecurity/charts/issues/481): >/help Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
R011y commented 1 year ago

WE CAN DO IT GUYS <3

poiana commented 11 months ago

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

jemag commented 11 months ago

/remove-lifecycle stale

leogr commented 11 months ago

Is anyone working on this? :thinking:

kek-Sec commented 11 months ago

this should be updated , its common for clusters to have

allowPrivilegeEscalation=false

poiana commented 8 months ago

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

poiana commented 7 months ago

Stale issues rot after 30d of inactivity.

Mark the issue as fresh with /remove-lifecycle rotten.

Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle rotten

jemag commented 7 months ago

/remove-lifecycle stale

mike-stewart commented 7 months ago

/remove-lifecycle rotten

poiana commented 4 months ago

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

mike-stewart commented 4 months ago

/remove-lifecycle stale

leogr commented 3 months ago

@alacuku @Issif any opinion on this?

Issif commented 3 months ago

It's not a so simple topic. Right now, the end user has to fill the whole securityContext object by him/herself. It's very flexible but requires some expertise and attempts.

  securityContext:
      securityContext:
        {{- include "falco.securityContext" . | nindent 8 }}

With Falco which must run in so many different contexts (on-prem, EKS, AKS, GKE, Openshift, Rancher, ...) this is the only method with allows enough flexibility.

jemag commented 3 months ago

out of curiosity, any reason Falco would need widely different securityContext between these environments? (on-prem, eks, aks, etc.)

jemag commented 3 months ago

for example, it seems improbable to me that falcosidekick would vary widely between these different environments. If that is the case, a strong default would make sense.

I could see falco itself needing different capabilities though.

Issif commented 3 months ago

I had in mind Falco, it might imply some tunning for Openshift for example. But I agree, for the simple apps like sidekick, it's not a big deal

poiana commented 2 weeks ago

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

mike-stewart commented 2 weeks ago

/remove-lifecycle stale