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

Implement fine-grained RBAC permssions to update/delete application resources #17991

Closed
agaudreault opened this issue Apr 26, 2024 · 1 comment · Fixed by #18124
Closed

Implement fine-grained RBAC permssions to update/delete application resources #17991

agaudreault opened this issue Apr 26, 2024 · 1 comment · Fixed by #18124
Assignees
Labels
component:api API bugs and enhancements component:rbac Issues related to Openshift and Racher enhancement New feature or request

Comments

@agaudreault
Copy link
Member

agaudreault commented Apr 26, 2024

Summary

The goal of this proposal is to allow fine-grained permission in the RBAC configuration for update and delete operations. It modifies the policy permission pattern based on Casbin to support subresource on a native delete and update operations.

Motivation

Developers with low knowledge of Kubernetes use Argo CD UI to interact with their applications. When developers need to operate their application, they sometimes have to modify or delete resources. In an ideal life, all changes would go through git, but sometimes a change needs to be made quickly to mitigate an issue.

Pod needs to be delete

A developer might need to delete a pod in ArgoCD UI to fix their application, which would be non-impactful because it would be immediately replaced by ReplicaSet or other mechanisms.

However, that user may not need to delete the Deployment resource or ReplicaSet, which would be Impactful due to application downtime.

To prevent any knid of errors and aim for the principle of least privilege, the user should only have permission to delete Pods in their Applciation as defined by the RBAC.

Patching manifest of specific resources

A developer might need to patch a manifest in ArgoCD UI to fix their application. However, that user may not need to update all the resources of their applications.

To prevent any kind of errors and aim for the principle of least privilege, the user should only have permission to update predefined manifest as defined by the RBAC.

Proposal

Terminology

Based on ArgoCD RBAC Documentation, we define a permission as p, <role/user/group>, <resource>, <action>, <appproject>/<object>.

Current State

Deleting an application's resource today

The permission necessary to delete a resource of an application is p, <role/user/group>, applications, delete, <appproject>/<app-name>, allow. This policy also give the user permission to delete the / Application.

RBAC Resources and Actions

Resources: clusters, projects, applications, applicationsets,
repositories, certificates, accounts, gpgkeys, logs, exec,
extensions

Actions: get, create, update, delete, sync, override,action/<group/kind/action-name>

Note that sync, override, and action/<group/kind/action-name> only have meaning for the applications resource.

Application resources

The resource path for application objects is of the form
<project-name>/<application-name>.

Delete access to sub-resources of a project, such as a rollout or a pod, cannot
be managed granularly. <project-name>/<application-name> grants access to all
subresources of an application.

Options

A) Add to the existing applications resource (preferred)

Pattern: p, <role/user/group>, applications, delete/<group>/<kind>/<ns>/<name>, <appproject>/<app-name>, allow

Application resources are part of the application API. So it is reasonable to use applications as the <resource>. When we want to delete the sub-resource, it can be argued that the <action> we are doing is delete/<group>/<kind>/<ns>/<name>.

B) Create a new resources resource

Pattern: p, <role/user/group>, resources, delete<group>/<kind>/<ns>/<name>, <appproject>/<app-name>, allow

Even if resources are part of the Application API details today, this can be considered as an implementation detail and does not need to be reflected in the policy. However, to keep backward compatibility, it may be unclear to the user that there are two existing <resource> that can be configured to allow deleting application resources.

C) Use a more specific <object>

Pattern: p, <role/user/group>, applications, delete, <appproject>/<app-name>/<group>/<kind>/<ns>/<name>, allow

The truth about this option is that the documentation is lying. When ArgoCD is configured in namespaced mode, the <object> is actually converted to <app-ns>/<app-name>. This creates a problem that could cause privilege escalation based on misconfiguration.

Supposing I want to allow developers to delete any resource in the bar namespace for app foo in the argocd namespace, I would define the policy as

  • p, developers, applications, delete, my-project/foo/bar/*

Unfortunately, we are now accidentally granting the developers access to delete any resources in any app named bar in the foo namespace.

For this reason, the <object> should not be extended to include a sub-resource.

Backward compatibility

The current DeleteResource API checks if user has the capabilities to applications, delete, <appproject>/<app-name>. If the user is allowed, it will delete the subresource with the k8s API. If it is not allowed, it will return a permission denied request.

With the new policy, the DeleteResource API would do the following

  1. check if user can applications, delete, <appproject>/<app-name>
    1. True: since the user has the permission to delete a whole application, and to maintain backward compatibility, the user has permission to delete any resource. 200 OK
    2. False: the API will perform a new call to applications, delete/<group>/<kind>/<ns>/<name>, <appproject>/<app-name>
      3. True: The user can delete the resource
      4. False: The user cannot delete the resource

The first validation would be the same as the current one. It validates if the user has permissions to the application

Security concerns

It should not be possible to infer if the Application exists or not by calling the API and looking at 1) the status code and B) the duration of the API call.

  1. check if App exist
    1. check if user can applications, delete, <appproject>/<app-name>
      1. True:
        1. App Exist: since the user has the permission to delete a whole application, and to maintain backward compatibility, the user has permission to delete any resource. return 200
        2. App does not exist: return 404 because the user has permission
      2. False:
        1. perform a new call to applications, delete/<group>/<kind>/<ns>/<name>, <appproject>/<app-name>, even if app doesn`t exist
          1. True:
            1. App Exist: 200
            2. App does not exist: 404
          2. False:
            1. App Exist: 403
            2. App does not exist: 403
  2. If 200 => try to delete the resource with argo-cd server Service Account.

Decision

Implementation of Option A.

Multiple proposals, discussions, opinions and suggestions have been brought up in other issues over the last months, and years. However, the no implementation were released to solve the issue.

Sometimes, we have to commit to something, even if it is not the ideal solution. I think the solution above is a good compromise for the user experience, security, migration impact and development time.

Long-term Alternatives

Project role impersonation of a Kubernetes service account is a viable alternative to this proposal, but requires a lot more changes to Argo. There seem to be a shift to go towards this solution in the long term. #9095

However, adding to the ArgoCD ARBAC does not increase or reduce the amount of work that will be required to support impersonation in the future.

Other alternatives

Allowing delete through actions could have been an Options, but it seemed like this option would require additional changes to the UI to give the same experience as the built-in option, and it would create 2 possible ways to delete a resource which could have been confusing to the users of ArgoCD UI or API without additional changes.

@agaudreault agaudreault added the enhancement New feature or request label Apr 26, 2024
@agaudreault agaudreault self-assigned this Apr 26, 2024
@agaudreault agaudreault added component:api API bugs and enhancements component:rbac Issues related to Openshift and Racher labels Apr 26, 2024
@agaudreault
Copy link
Member Author

agaudreault commented Apr 26, 2024

Related to #7689 #9095 #12777 #3593 #6295 #14379 #18058

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:api API bugs and enhancements component:rbac Issues related to Openshift and Racher enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant