-
Notifications
You must be signed in to change notification settings - Fork 155
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
Limit Default RBAC Rules For Kanister Operator #3134
Conversation
Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>
Can you please add the docs changes as well? |
Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>
@viveksinghggits I've Added. Please take a look |
Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>
Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>
Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@r4rajat We should make changes in docs_new
as well and add a release note too.
docs/install.rst
Outdated
.. note:: | ||
Kanister might not be able to take backups or restore data out of the box, as it | ||
requires permissions to access the resources in the cluster. You will need to configure | ||
the necessary permissions for Kanister to work correctly. | ||
For RBAC configuration, please refer to the :ref:`RBAC Configuration <rbac>` section. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should try to rephrase it, maybe something like below
To enhance the security of the cluster, default installation of Kanister doesn't have access
to any resource in other namespaces apart from where it's installed. Because of this Kanister
might not be able to snapshot or restore the applications by default. If the Blueprint requires access to the resources in other namespaces, please follow the steps here to configure the access properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please feel free to change above suggestion to make it better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@viveksinghggits I've rephrased your suggestion, LMK if it looks good.
To improve the cluster's security, the default installation of Kanister is restricted to access only the resources within its own namespace. As a result, Kanister may not be able to snapshot or restore applications by default in other namespaces. If Blueprint needs access to resources in other namespaces, please follow the steps provided here to configure the access correctly.
To allow Kanister to perform backup/restore operations in a specific | ||
namespace, create a `RoleBinding` that assigns the `edit` `ClusterRole` | ||
to Kanister's Service Account: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should mentioned that edit
is a system created cluster role to make sure users don't confused about where did edit
cluster role come from.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
docs/rbac.rst
Outdated
If you prefer to fine-tune the access, you can create a `Role` with | ||
specific permissions and bind it to Kanister's Service Account: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should mention that if your blueprint doesn't require access to all the resources that are mentioned in edit
cluster role, you can create the role with just the resources/verbs that the blueprint requires access to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
docs/rbac.rst
Outdated
To allow Kanister to perform backup/restore operations in a specific | ||
namespace, create a `RoleBinding` that assigns the `edit` `ClusterRole` | ||
to Kanister's Service Account: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To allow Kanister to perform backup/restore operations in a specific | |
namespace, create a `RoleBinding` that assigns the `edit` `ClusterRole` | |
to Kanister's Service Account: | |
To allow Kanister to perform backup/restore operations in an application | |
namespace, create a `RoleBinding` in the application namespace that associates the `edit` `ClusterRole` | |
to Kanister's Service Account: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>
Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>
docs/rbac.rst
Outdated
The `edit` role is a built-in Kubernetes system role that provides permissions | ||
to modify most objects within a namespace, excluding roles, role bindings, and | ||
resource quotas. It allows access to create, update, delete, and view resources | ||
such as Deployments, Pods, Services, ConfigMaps, PersistentVolumeClaims, and more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good but I think we should talk about it when we are saying edit
cluster role has been removed in above section.
Maybe immediately after RBAC Configuration
, after explaining what edit
cluster role is, we can say that kanister's service account used to get associated with the kaniter service account which doesn't happen anymore.
And I like how you have reformatted the above paragraphs based on my suggestion so please do that with above suggestion as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you check once @viveksinghggits
docs/rbac.rst
Outdated
Creating a RoleBinding with edit ClusterRole | ||
============================================ | ||
|
||
The `edit` role is a built-in Kubernetes system role that provides permissions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
edit
is a cluster role right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, will update that
docs/rbac.rst
Outdated
in application namespace to bind the `Role` to Kanister's Service Account. | ||
This approach enhances security by granting only the necessary permissions. | ||
|
||
1. Create a `Role` with the required permissions: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1. Create a `Role` with the required permissions: | |
1. Create a `Role` with the permissions required by your blueprint: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done !
|
||
kubectl create rolebinding kanister-role-binding --role=kanister-role \ | ||
--serviceaccount=<release-namespace>:<release-name>-kanister-operator \ | ||
--namespace=<application-namespace> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a line after this, saying after doing this kanister would be able to snapshot/restore the namespace successfully.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done !
- Encouraged the adoption of least privilege principles by allowing users to create roles with only the necessary permissions. | ||
fixes: | ||
- Fixed inconsistencies in RBAC configuration that allowed Kanister to have broader permissions than required. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these two can be removed? because we are able to convey enough information using the upgrade
and security
notes.
Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>
Co-authored-by: Vivek Singh <vsingh.ggits.2010@gmail.com>
Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>
Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>
Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>
Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
We should see if someone can have a quick look, otherwise I will approve.
docs/rbac.rst
Outdated
Earlier, the Kanister operator was bound to the `edit` `ClusterRole` | ||
using a `ClusterRoleBinding` provided by the Helm Chart. | ||
|
||
The `edit` `ClusterRole` is a built-in Kubernetes system role that offers | ||
permissions to modify most objects within a namespace, excluding roles, | ||
role bindings, and resource quotas. This role allowed access to create, | ||
update, delete, and view resources such as Deployments, Pods, Services, | ||
ConfigMaps, PersistentVolumeClaims, and more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Earlier, the Kanister operator was bound to the `edit` `ClusterRole` | |
using a `ClusterRoleBinding` provided by the Helm Chart. | |
The `edit` `ClusterRole` is a built-in Kubernetes system role that offers | |
permissions to modify most objects within a namespace, excluding roles, | |
role bindings, and resource quotas. This role allowed access to create, | |
update, delete, and view resources such as Deployments, Pods, Services, | |
ConfigMaps, PersistentVolumeClaims, and more. | |
The `edit` `ClusterRole` is a built-in Kubernetes system role that offers | |
permissions to modify most objects within a namespace, excluding roles, | |
role bindings, and resource quotas. This role allowed access to create, | |
update, delete, and view resources such as Deployments, Pods, Services, | |
ConfigMaps, PersistentVolumeClaims, and more. Kanister helm chart used | |
assign `edit` ClusterRole to the Kanister service account which gave all the | |
permissions mentioned in the `edit` ClusterRole to the Kanister application. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done !
upgrade: | ||
- Users upgrading from previous versions should note that the `edit` `ClusterRoleBinding` is no longer included by default. They must now create their own `Role` / `RoleBinding` with appropriate permissions for Kanister's Service Account in the application's namespace. | ||
security: | ||
- Improved security by removing the default `edit` `ClusterRoleBinding` assignment, reducing the risk of over-permissioning. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is over-permisssioning
correct english work? 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated !
Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>
Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
@r4rajat One question: did you test the helm chart upgrade? Would it delete existing binding when upgraded?
@hairyhum I tested it by
|
Change Overview
Current default RBAC role bindings for kanister operator allow creation of pods in any namespaces with any service accounts, thus enabling privilege escalations.
Kanister should adopt safer default, such as only allowing to create pods in the kanister namespace.
Current Docs Screenshots
New Docs Screenshot
Pull request type
Please check the type of change your PR introduces:
Issues
Test Plan
Manual Testing
1) Install Kanister and MySQL
2) Create Blueprint, Profile and ActionSet to Backup mysql StatefulSet
With Updated Permissions to Kanister Operator, It SHOULD NOT be able to create any
Pod
as part ofKubeTask
Function inmysql
Namespace.3) How to take Backup Now ?
Create a Role in
mysql
Namespace withedit
ClusterRole and assign it to Kanister Operator's ServiceAccountTake Backup Again
Check Status