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.

Sign up for GitHub

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

Jump to bottom

Forbid container privilege escalations for ETCD Druid component containers #1038

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Forbid container privilege escalations for ETCD Druid component containers #1038

wants to merge 4 commits into from
+28 −9

Conversation

georgibaltiev
Copy link

How to categorize this PR?

/area security
/area compliance
/kind enhancement

What this PR does / why we need it:
This PR sets securityContext.allowPrivilegeEscalation to false for every Seed and Garden cluster component container, which does not have securityContext.Privileged set to true or one of CAP_SYS_ADMIN/SYS_ADMIN capabilities added.

**Which issue(s) this PR
Part of gardener/gardener#11139

Special notes for your reviewer:
cc @AleksandarSavchev @ialidzhikov

Release note:

Component containers, which do not require privilege escalations, now forbid privilege escalation explicitly.

@gardener-prow Gardener Prow
Copy link

gardener-prow bot commented Mar 17, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@gardener-robot gardener-robot added area/compliance Compliance related area/security Security related kind/enhancement Enhancement, improvement, extension needs/review Needs review size/s Size of pull request is small (see gardener-robot robot/bots/size.py) labels Mar 17, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 17, 2025
@gardener-robot-ci-3 gardener-robot-ci-3 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 17, 2025
@georgibaltiev georgibaltiev force-pushed the forbid-container-privilege-escalations branch from 64a3aa3 to 566038c Compare March 17, 2025 15:16
@gardener-robot gardener-robot added size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) and removed size/s Size of pull request is small (see gardener-robot robot/bots/size.py) labels Mar 17, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 17, 2025
@gardener-robot gardener-robot added size/s Size of pull request is small (see gardener-robot robot/bots/size.py) and removed size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) labels Mar 18, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 18, 2025
@georgibaltiev georgibaltiev marked this pull request as ready for review March 18, 2025 09:16
@georgibaltiev georgibaltiev requested a review from a team as a code owner March 18, 2025 09:16
@shreyas-s-rao shreyas-s-rao added this to the v0.29.0 milestone Mar 19, 2025
@anveshreddy18
Copy link
Contributor

@georgibaltiev, thanks for making the change to set the securityContext.allowPrivilegeEscalation to false. Can you pls resolve the conflicts so we can review and merge the PR.

@georgibaltiev georgibaltiev force-pushed the forbid-container-privilege-escalations branch from 1e5eca0 to 66c2e63 Compare March 24, 2025 07:01
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 24, 2025
@georgibaltiev
Copy link
Author

/test pull-etcd-druid-e2e-kind

@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 24, 2025
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 24, 2025
@georgibaltiev
Copy link
Author

/test pull-etcd-druid-integration

@georgibaltiev
Copy link
Author

/retest-required

@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 24, 2025
renormalize
renormalize requested changes Mar 28, 2025
View reviewed changes
Copy link
Member

@renormalize renormalize left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for the slow review @georgibaltiev!

internal/component/statefulset/builder.go
RunAsGroup: ptr.To[int64](0),
RunAsNonRoot: ptr.To(false),
RunAsUser: ptr.To[int64](0),
AllowPrivilegeEscalation: ptr.To(false),
Copy link
Member

@renormalize renormalize Mar 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this container is already running as root (is deployed only during tests), don't think setting this flag here would do anything. can this just be removed? please correct me if I'm wrong

Suggested change
AllowPrivilegeEscalation: ptr.To(false),

internal/controller/etcdcopybackupstask/reconciler.go
RunAsGroup: ptr.To[int64](0),
RunAsNonRoot: ptr.To(false),
RunAsUser: ptr.To[int64](0),
AllowPrivilegeEscalation: ptr.To(false),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1038 (comment)

Suggested change
AllowPrivilegeEscalation: ptr.To(false),

@gardener-robot gardener-robot added the needs/changes Needs (more) changes label Mar 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@renormalize renormalize

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
area/compliance Compliance related area/security Security related kind/enhancement Enhancement, improvement, extension needs/changes Needs (more) changes needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/review Needs review size/s Size of pull request is small (see gardener-robot robot/bots/size.py)
Projects
None yet
Milestone
v0.29.0
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@georgibaltiev @anveshreddy18 @renormalize @gardener-robot-ci-1 @shreyas-s-rao @gardener-robot-ci-2 @gardener-robot-ci-3 @gardener-robot