Skip to content

Conversation

Camelron
Copy link
Contributor

@Camelron Camelron commented Apr 29, 2025

Handle PodSecurityContext.fsGroup|supplementalGroups

Policy enforcement for additionalGids, A list of groups applied to the first process run in each container. Manifests in OCI struct as additionalGids: Consists of container's GID, fsGroup, and supplementalGroups.

See: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#PodSecurityContext-v1-core

With added support for parsing these fields in genpolicy, we can now enable policy verification of AdditionalGids.

@katacontainersbot katacontainersbot added the size/medium Average sized task label Apr 29, 2025
@Camelron Camelron force-pushed the cameronbaird/address-gid-mismatch-additionalgids branch from 11150ff to 913ebe6 Compare May 12, 2025 23:51
@katacontainersbot katacontainersbot added size/large Task of significant size and removed size/medium Average sized task labels May 12, 2025
@Camelron Camelron force-pushed the cameronbaird/address-gid-mismatch-additionalgids branch from 913ebe6 to 2b200c2 Compare May 13, 2025 21:34
@katacontainersbot katacontainersbot added size/huge Largest and most complex task (probably needs breaking into small pieces) and removed size/large Task of significant size labels May 13, 2025
@Camelron Camelron marked this pull request as ready for review May 13, 2025 21:36
Policy enforcement for additionalGids, A list of groups applied to the first process run in each container.

Manifests in OCI struct as additionalGids: Consists of container's GID, fsGroup, and supplementalGroups.

https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#PodSecurityContext-v1-core

Signed-off-by: Cameron Baird <cameronbaird@microsoft.com>
@sprt sprt requested review from burgerdev and Redent0r May 13, 2025 21:47
Camelron added 3 commits May 13, 2025 21:48
With added support for parsing these fields in genpolicy, we can now
enable policy verification of AdditionalGids.

Signed-off-by: Cameron Baird <cameronbaird@microsoft.com>
Introduce new test case to the security context bats file which verifies
that policy works properly for a deployment yaml containing fsGroup and
supplementalGroup configuration.

Signed-off-by: Cameron Baird <cameronbaird@microsoft.com>
Fix up genpolicy test inputs to include required additionalGids

Include a test for the pod_container container in security_context tests
as these containers follow slightly different paths in containerd.

Introduce a test for fsGroup/supplementalGroups fields in the security
context.

Signed-off-by: Cameron Baird <cameronbaird@microsoft.com>
@Camelron Camelron force-pushed the cameronbaird/address-gid-mismatch-additionalgids branch from 2b200c2 to 090497f Compare May 13, 2025 21:49
Copy link
Contributor

@Redent0r Redent0r left a comment

Choose a reason for hiding this comment

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

lgtm. Thanks @Camelron !

Copy link
Contributor

@burgerdev burgerdev left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

As the genpolicy from_files call makes network requests to container
registries, it has a chance to fail.

Harden us against flakes due to network by introducing a 6x retry loop
in genpolicy tests.

Signed-off-by: Cameron Baird <cameronbaird@microsoft.com>
@Camelron Camelron force-pushed the cameronbaird/address-gid-mismatch-additionalgids branch from 66f17fc to 7bba737 Compare May 15, 2025 18:12
Copy link
Member

@danmihai1 danmihai1 left a comment

Choose a reason for hiding this comment

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

Thanks for solving these issues @Camelron !

@danmihai1 danmihai1 merged commit b9651ea into kata-containers:main May 16, 2025
370 of 386 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test size/huge Largest and most complex task (probably needs breaking into small pieces)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants