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

[RFE] Add webhook escalation check for "NamespacedRules" field #42216

Closed
samjustus opened this issue Jul 25, 2023 · 4 comments
Closed

[RFE] Add webhook escalation check for "NamespacedRules" field #42216

samjustus opened this issue Jul 25, 2023 · 4 comments
Assignees
Labels
area/rbac JIRA To be used in correspondence with the internal ticketing system. kind/enhancement Issues that improve or augment existing functionality priority/1 QA/L team/collie the team that is responsible for auth and rbac within rancher
Milestone

Comments

@samjustus
Copy link
Collaborator

samjustus commented Jul 25, 2023

related to Global Roles V2

SURE-6636

@samjustus samjustus added kind/enhancement Issues that improve or augment existing functionality team/area1 JIRA To be used in correspondence with the internal ticketing system. labels Jul 25, 2023
@samjustus samjustus added this to the 2023-Q4-v2.8x milestone Jul 25, 2023
@samjustus
Copy link
Collaborator Author

blocked by #42215

@MbolotSuse
Copy link
Contributor

Basic Requirements

  • The webhook should prevent users from modifying the owner label on role/rolebinding objects
    • The webhook checks for these built-in types should use a label selector (example) to ensure that we only validate roles/rolebindings which have this label
    • These webhook checks should only run in the local cluster (i.e. when MCM is enabled)
  • Escalation checks should be made for each namespace that the globalRole grants permissions in.
    • When determining current user permissions in a given namespace, the rules from that namespace in NamespaceRules and the rules active in the global context in Rules should both be used

@JonCrowther
Copy link
Contributor

Validation

Root Cause

This is additional follow up work from #42215. With the new rancher functionality, we also need some new webhook changes to accommodate them.

What was fixed, or what change has occurred

The first change was to add escalation checks for the NamespacedRules field. Users can't create GRs or assign GRBs that have privileges greater than their own.

The second change is to prevent users from removing or modifying the growner and grbownerlabels from roles and role bindings respectively. Doing so would cause disruption in how we manage them.

Areas or cases that should be tested

To test the above changes, there should be tests related to assigning or creating GRs/GRBs that have privileges greater than the user currently has. The exception being if they have the escalate or bind verb (users with * have escalate/bind). These users can provide any rules that they would like.

The second test would be attempting to modify or remove the growner and grbowner labels. That should be blocked by the webhook.

What areas could experience regressions

The changes affect GR and GRB validation, so that would be the most likely area for regressions. Inherited cluster rules and global rules should work exactly the same as before.

New validators were added to Roles and RoleBindings. It should only apply to the growner and grbowner labels, so any issues regarding Roles and RoleBindings being rejected by the webhook would be regressions.

@MKlimuszka MKlimuszka modified the milestones: v2.8-Next1, v2.9.0 Jan 8, 2024
@MKlimuszka MKlimuszka modified the milestones: v2.9.0, v2.9-Next1 Jan 18, 2024
@samjustus samjustus added team/collie the team that is responsible for auth and rbac within rancher and removed team/area1 labels Feb 1, 2024
@anupama2501
Copy link
Contributor

Validated on v2.9-head 3c4ccdc5

Following checks were validated:

Test case Status
  Verify passing empty verbs in the values of a namespace under namespaced rules. PASS 
  Verify passing incorrect verbs in the values of a namespace under namespaced rules. PASS 
  Verify adding incorrect resources does not give access to any other resource for the user. PASS 
  Create a global role binding manually by adding the owner label of an existing grb PASS 
  As an admin, verify editing the label throws an error. PASS 
  Rancher server restart, verify no permission errors noticed in the rancher logs. PASS 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/rbac JIRA To be used in correspondence with the internal ticketing system. kind/enhancement Issues that improve or augment existing functionality priority/1 QA/L team/collie the team that is responsible for auth and rbac within rancher
Projects
None yet
Development

No branches or pull requests

5 participants