Skip to content

docs: clarify RBAC trust model for cluster-scoped CRs - #2335

Open
dgn wants to merge 1 commit into
istio-ecosystem:mainfrom
dgn:check-namespace-perms
Open

dgn wants to merge 1 commit into
istio-ecosystem:mainfrom
dgn:check-namespace-perms

Conversation

@dgn

@dgn dgn commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Creating/updating Istio, IstioRevision, IstioCNI, or ZTunnel resources is equivalent to cluster-admin, since the operator's own ServiceAccount can act on any namespace on the caller's behalf.

Fixes #2114.

@dgn
dgn requested a review from a team as a code owner September 15, 2026 13:12
@dgn
dgn force-pushed the check-namespace-perms branch from a8f1070 to 103b7e0 Compare September 15, 2026 13:13
Creating/updating Istio, IstioRevision, IstioCNI, or ZTunnel resources is equivalent to cluster-admin, since the operator's own ServiceAccount can act on any namespace on the caller's behalf.

Signed-off-by: Daniel Grimm <dgrimm@redhat.com>
@dgn
dgn force-pushed the check-namespace-perms branch from 103b7e0 to 366222f Compare September 15, 2026 13:15
@codecov

codecov Bot commented Sep 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.80%. Comparing base (f331eb2) to head (366222f).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2335      +/-   ##
==========================================
- Coverage   87.95%   87.80%   -0.15%     
==========================================
  Files          56       56              
  Lines        2723     2723              
==========================================
- Hits         2395     2391       -4     
- Misses        328      332       +4     
Flag Coverage Δ
integration-tests 73.54% <ø> (-0.29%) ⬇️
unit-tests 67.57% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dgn

dgn commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator Author

/retest

1 similar comment
@dgn

dgn commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator Author

/retest

@FilipB FilipB left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

While the documentation is clear we are still not forcing this as the issue is suggesting. It's probably not worth the effort as anyone who is able to create/edit cluster-wide resources should be consider as cluster admin? There is no room for error in this case?

@dgn

dgn commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator Author

/retest

@dgn

dgn commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator Author

While the documentation is clear we are still not forcing this as the issue is suggesting. It's probably not worth the effort as anyone who is able to create/edit cluster-wide resources should be consider as cluster admin? There is no room for error in this case?

From my perspective, we have two options:
a) continue with the current policy of simply accepting that access to sail CRs equals cluster-admin access
b) build a complex policy engine that verifies for each CR edit that the user in question does actually have permissions for all changes that will be applied in the cluster

I think to fully solve the issue, b) is the only option, but I'm not sure it's worth the effort, or even really viable*. Giving these sorts of guarantees would also mean that every possible privilege escalation is a critical bug in Sail Operator, creating a lot of work for us.

*: we could e.g. only verify this at edit-time using a webhook. At reconciliation time, we simply cannot know which user made which changes to the CR. So effectively, any user with edit permission on webhook configs can circumvent the checks and use Sail Operator for privilege escalation.

@FilipB

FilipB commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

While the documentation is clear we are still not forcing this as the issue is suggesting. It's probably not worth the effort as anyone who is able to create/edit cluster-wide resources should be consider as cluster admin? There is no room for error in this case?

From my perspective, we have two options: a) continue with the current policy of simply accepting that access to sail CRs equals cluster-admin access b) build a complex policy engine that verifies for each CR edit that the user in question does actually have permissions for all changes that will be applied in the cluster

I think to fully solve the issue, b) is the only option, but I'm not sure it's worth the effort, or even really viable*. Giving these sorts of guarantees would also mean that every possible privilege escalation is a critical bug in Sail Operator, creating a lot of work for us.

*: we could e.g. only verify this at edit-time using a webhook. At reconciliation time, we simply cannot know which user made which changes to the CR. So effectively, any user with edit permission on webhook configs can circumvent the checks and use Sail Operator for privilege escalation.

Thank you. Option a) seems good enough.
Approving and adding hold so others can also review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[SECURITY] No admission webhook validates spec.namespace, allowing namespace escalation

2 participants