User Impersonation #418
User Impersonation #418
Conversation
aadding section outlinning bringing in origin code.
| We will need to add some security settings to help administrators lock down the broker. For this, we are going to check for privilege escalation and will move the location of where the APB pod will run. | ||
|
|
||
| ## Problem Description | ||
| The problem is that currently privilege escalation is a concern for users who have access to the broker. We want to give the cluster admin who is setting up the service broker to have assurances that the broker can have some safety assurances. |
There was a problem hiding this comment.
last sentence is a little wordy
| We will need to do 5 things to satisfy the requirements. | ||
| 1. Check if the user has the permissions to cover the cluster role to be used. | ||
| 2. If the cluster admin while setting up the broker has set a config value to auto escalate we will immediately continue and not check the user's permissions. | ||
| 3. Create transient namespace/project |
There was a problem hiding this comment.
Instead of a transient namespace, should we create an APB only namespace that only can be accessed by the broker? Do we gain anything by deleting and creating a namespace per APB?
There was a problem hiding this comment.
I think we can an extra layer of isolation. In practice, I don't know if that matters. I could see this being a configuration value down the road if you don't want the overhead.
There was a problem hiding this comment.
We were advised to proceed with the transient namespace per aos-security, ensures cleaner workflow.
Making this a config option in future sound good, that is not a requirement for 3.7 timeframe.
|
|
||
| ## Work Items | ||
| - Create a new config value called auto escalate. | ||
| - Use the [SubjectRulesReview](https://docs.openshift.org/latest/rest_api/apis-authorization.openshift.io/v1.SubjectRulesReview.html) API to retrieve the rules for a user. |
There was a problem hiding this comment.
Looks like there's no kubernetes equivalent just yet: kubernetes/kubernetes#31292 . Since we're adding Kubernetes support to the broker, we're going to need to find away to make this work or disable it until 1.8 has this feature.
There was a problem hiding this comment.
@jwmatthews have any thoughts? I think the answer is that if we are going to work on kubernetes that we you must opt into the auto-escalation?
There was a problem hiding this comment.
@shawn-hurley agree
Let's implement a sanity check during startup for configuration options.
If "auto_escalate: False" is set on a pure kubernetes run then the Broker should log the issue (unsupported configuration option on pure k8s) error out and die. I'm envisioning we'd perform this check during initial startup of broker and error out immediately if we are configured for an option we can not support.
@shawn-hurley please file a new issue on our broker to track the problem running on k8s lacking needed APIs for "auto_escalate: False", include the link @rthallisey provided as a required dependency, kubernetes/kubernetes#31292
| The current proposal will achieve the re-use of the conversion and cover methods from origin by copying files to the broker. This particular path will require vendor being updated. This also means that we **not** be vendoring all of origin to get the functions that we need, but will be making copies. | ||
|
|
||
| #### Pros Of Not Vendoring Origin | ||
| * The broker still remains independent of origin. W |
| The problem is that currently privilege escalation is a concern for users who have access to the broker. We want to give the cluster admin who is setting up the service broker the ability to not allow users to have privilege escalation. | ||
|
|
||
| ## Implementation Details | ||
| We will need to do 5 things to satisfy the requirements. |
| ## Implementation Details | ||
| We will need to do 5 things to satisfy the requirements. | ||
| 1. Check if the user has the permissions to cover the cluster role to be used. | ||
| 2. If the cluster admin while setting up the broker has set a config value to auto escalate we will immediately continue and not check the user's permissions. |
There was a problem hiding this comment.
I would combine 1+2, and make it something like
Check if the user has the permissions to cover the cluster role to be used. If the auto-escalate option is enabled in the broker config, we will immediately continue and not check the user's permissions.
|
|
||
|
|
||
| ## Of Importance To Note | ||
| The current proposal will achieve the re-use of the conversion and cover methods from origin by copying files to the broker. This particular path will require vendor being updated. This also means that we **not** be vendoring all of origin to get the functions that we need, but will be making copies. |
There was a problem hiding this comment.
How many files/functions will need to be copied?
There was a problem hiding this comment.
28 files like 56 functions and 41 structures, Most of the functions are unused and we could eliminate them, but I think they may be useful later for validation of openshift rules/users/roles
| We will need to do 5 things to satisfy the requirements. | ||
| 1. Check if the user has the permissions to cover the cluster role to be used. If the `auto_escalate` option is enabled in the broker config, we will immediately continue and not check the user's permissions. | ||
| 2. If the cluster admin while setting up the broker has set a config value to auto escalate we will immediately continue and not check the user's permissions. | ||
| 3. Create transient namespace/project |
There was a problem hiding this comment.
nitpick: should end this list item with period (like the others).
| ## Implementation Details | ||
| We will need to do 5 things to satisfy the requirements. | ||
| 1. Check if the user has the permissions to cover the cluster role to be used. If the `auto_escalate` option is enabled in the broker config, we will immediately continue and not check the user's permissions. | ||
| 2. If the cluster admin while setting up the broker has set a config value to auto escalate we will immediately continue and not check the user's permissions. |
There was a problem hiding this comment.
Delete this (and you'll have five 😉) or remove the snippet above talking about auto_escalate. Either way, use auto_escalate when referring to that config option.
| - Create a new config value called auto escalate. | ||
| - Use the [SubjectRulesReview](https://docs.openshift.org/latest/rest_api/apis-authorization.openshift.io/v1.SubjectRulesReview.html) API to retrieve the rules for a user. | ||
| - Use the [k8s API's](https://godoc.org/k8s.io/client-go/kubernetes/typed/rbac/v1#ClusterRoleInterface) to retrieve the [rules](https://godoc.org/k8s.io/api/rbac/v1#PolicyRule) from the [cluster role](https://godoc.org/k8s.io/api/rbac/v1#ClusterRole). | ||
| - Use the conversion from origin to convert to the same type of rules |
There was a problem hiding this comment.
nit. be consistent with list item endings (use .)
Describe what this PR does and why we need it:
Proposal for implementing user impersonation