Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide good container securityContext by default for each chart #481

Open
jemag opened this issue Apr 13, 2023 · 30 comments
Open

Provide good container securityContext by default for each chart #481

jemag opened this issue Apr 13, 2023 · 30 comments
Labels
help wanted Extra attention is needed kind/feature New feature or request

Comments

@jemag
Copy link

jemag commented Apr 13, 2023

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 :

# -- Sidekick container securityContext
securityContext: {}

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:

securityContext:
{}
# capabilities:
# drop:
# - ALL
# readOnlyRootFilesystem: true
# runAsNonRoot: true
# runAsUser: 1000

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 falcosecurity/falco#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 jemag added the kind/feature New feature or request label Apr 13, 2023
@jemag
Copy link
Author

jemag commented Apr 13, 2023

This would cover and close #425 as well

@leogr
Copy link
Member

leogr commented May 2, 2023

@therealbobo
Copy link
Contributor

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
Copy link
Member

leogr commented May 2, 2023

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
Copy link
Author

jemag commented May 2, 2023

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):

securityContext:
{}
# capabilities:
# drop:
# - ALL
# readOnlyRootFilesystem: true
# runAsNonRoot: true
# runAsUser: 1000

@therealbobo
Copy link
Contributor

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
Copy link
Contributor

poiana commented Jul 31, 2023

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
Copy link
Author

jemag commented Jul 31, 2023

/remove-lifecycle stale

@R011y
Copy link

R011y commented Sep 1, 2023

Any update here?

@leogr
Copy link
Member

leogr commented Sep 5, 2023

/remove-lifecycle stale

cc @alacuku

@leogr
Copy link
Member

leogr commented Sep 5, 2023

/help

@poiana
Copy link
Contributor

poiana commented Sep 5, 2023

@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:

/help

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@poiana poiana added the help wanted Extra attention is needed label Sep 5, 2023
@R011y
Copy link

R011y commented Sep 5, 2023

WE CAN DO IT GUYS <3

@poiana
Copy link
Contributor

poiana commented Dec 4, 2023

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
Copy link
Author

jemag commented Dec 4, 2023

/remove-lifecycle stale

@leogr
Copy link
Member

leogr commented Dec 5, 2023

Is anyone working on this? 🤔

@kek-Sec
Copy link

kek-Sec commented Dec 7, 2023

this should be updated , its common for clusters to have

allowPrivilegeEscalation=false

@poiana
Copy link
Contributor

poiana commented Mar 6, 2024

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
Copy link
Contributor

poiana commented Apr 5, 2024

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
Copy link
Author

jemag commented Apr 5, 2024

/remove-lifecycle stale

@mike-stewart
Copy link
Contributor

/remove-lifecycle rotten

@poiana
Copy link
Contributor

poiana commented Jul 4, 2024

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
Copy link
Contributor

/remove-lifecycle stale

@leogr
Copy link
Member

leogr commented Jul 18, 2024

@alacuku @Issif any opinion on this?

@Issif
Copy link
Member

Issif commented Jul 18, 2024

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
Copy link
Author

jemag commented Jul 18, 2024

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

@jemag
Copy link
Author

jemag commented Jul 18, 2024

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
Copy link
Member

Issif commented Jul 18, 2024

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
Copy link
Contributor

poiana commented Oct 16, 2024

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
Copy link
Contributor

/remove-lifecycle stale

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed kind/feature New feature or request
Projects
None yet
Development

No branches or pull requests

8 participants