Skip to content

Harden bridge-marker SCC: restrict privileges, enforce non-root, allow projected volumes #2336

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

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

Conversation

RamLavi
Copy link
Collaborator

@RamLavi RamLavi commented Jun 5, 2025

What this PR does / why we need it:
This PR improves the security posture of the bridge-marker DaemonSet in OpenShift by hardening its SecurityContextConstraints (SCC) and enforcing best practices.

Special notes for your reviewer:
To anticipate reviewers request, commented on the PR why I did NOT remove other SCCs.

Release note:

reduce bridge-marker SCC

- Disallow hostDir volume plugins.
- Restrict volume types to configMap and emptyDir.
- Restrict volumes access to basic folders. Projected volume is required
for ServiceAccount token mounts.

Signed-off-by: Ram Lavi <ralavi@redhat.com>
@kubevirt-bot kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Jun 5, 2025
@kubevirt-bot kubevirt-bot requested a review from oshoval June 5, 2025 13:26
@kubevirt-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign qinqon for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubevirt-bot kubevirt-bot requested a review from phoracek June 5, 2025 13:26
@RamLavi
Copy link
Collaborator Author

RamLavi commented Jun 5, 2025

/hold
want to check this on kcli

@kubevirt-bot kubevirt-bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/M labels Jun 5, 2025
@RamLavi
Copy link
Collaborator Author

RamLavi commented Jun 5, 2025

🔒 SCC Non-Default Capabilities Review

Capability Set To Default Reason
allowHostNetwork true false Required to inspect the host network namespace to discover bridges.
readOnlyRootFilesystem true false (or unset) Harden container filesystem; application does not need write access.
runAsUser.type MustRunAsNonRoot RunAsAny Enforce non-root user for least privilege; safer runtime isolation.
seLinuxContext.type MustRunAs RunAsAny Enforce confined SELinux type to ensure MCS labeling and pod isolation.
volumes configMap, emptyDir, projected "*" (unrestricted) Limit volume types; prevent access to host filesystems (hostPath not allowed). projected is required for ServiceAccount tokens.

RamLavi added 3 commits June 10, 2025 09:55
As a preliminary step to the next commit where runtime dependencies are
hardened, mounting /tmp. This is required for compatibility with Go
libraries and client-go when using readOnlyRootFilesystem.
This commit mounts an emptyDir at /tmp to provide writable scratch space
for the container.

This change should also occur on the repo itself, but until it does, and
in order to allow for smooth bump when it occurs - it should stay.

Signed-off-by: Ram Lavi <ralavi@redhat.com>
- Enable readOnlyRootFilesystem for better container isolation.
- Require containers to run as non-root users.
- Set SELinux context type to MustRunAs for confinement.

Signed-off-by: Ram Lavi <ralavi@redhat.com>
…d-only rootfs

- Configure container to run as non-root user explicitly (UID 1001).
- Enable read-only root filesystem at container level.
- Ensure consistent non-root enforcement across architectures.

This change should also occur on the repo itself, but until it does, and
in order to allow for smooth bump when it occurs - it should stay.

Signed-off-by: Ram Lavi <ralavi@redhat.com>
@RamLavi RamLavi force-pushed the birdge_marker_reduce_scc branch from 946e952 to 7ab3e0b Compare June 10, 2025 07:16
Copy link

@RamLavi
Copy link
Collaborator Author

RamLavi commented Jun 10, 2025

checked on kcli and it runs smoothly:

[root@hades04 cluster-network-addons-operator]# oc get pod -n cluster-network-addons -owide | grep bridge-marker
bridge-marker-77n7k                                1/1     Running   0          11m   192.168.122.222   multi-homing-worker-0.yourname.corp   <none>           <none>
bridge-marker-bw4jp                                1/1     Running   0          11m   192.168.122.223   multi-homing-worker-1.yourname.corp   <none>           <none>
[root@hades04 cluster-network-addons-operator]# oc debug node/multi-homing-worker-0.yourname.corp -- chroot /host sudo ip link add br_test5 type bridge
Starting pod/multi-homing-worker-0yournamecorp-debug-cxf6s ...
To use host binaries, run `chroot /host`

Removing debug pod ...
[root@hades04 cluster-network-addons-operator]# oc debug node/multi-homing-worker-0.yourname.corp -- chroot /host sudo ip link set br_test5 up
Starting pod/multi-homing-worker-0yournamecorp-debug-hc56w ...
To use host binaries, run `chroot /host`

Removing debug pod ...
[root@hades04 cluster-network-addons-operator]# 
[root@hades04 cluster-network-addons-operator]# 
[root@hades04 cluster-network-addons-operator]# until oc get node multi-homing-worker-0.yourname.corp -o json | jq -e '.status.allocatable["bridge.network.kubevirt.io/br_test5"]' > /dev/null; do echo "Waiting for br_test5..."; sleep 5; done; echo "br_test5 is now available!"
Waiting for br_test5...
Waiting for br_test5...
br_test5 is now available!

@RamLavi
Copy link
Collaborator Author

RamLavi commented Jun 10, 2025

/hold cancel

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dco-signoff: yes Indicates the PR's author has DCO signed all their commits. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants