Prevent escalating resource access by default #8818
Conversation
| @@ -188,8 +209,59 @@ func (e clusterRoleEvaluator) ResolveRules(scope, namespace string, clusterPolic | |||
|
|
|||
| rules := []authorizationapi.PolicyRule{} | |||
| for _, rule := range role.Rules { | |||
| // rules with unbounded access shouldn't be allowed in scopes. | |||
| if rule.Verbs.Has(authorizationapi.VerbAll) || rule.Resources.Has(authorizationapi.ResourceAll) || getAPIGroupSet(rule).Has(authorizationapi.APIGroupAll) { | |||
There was a problem hiding this comment.
I think the escalating check should apply to these as well
There was a problem hiding this comment.
I think the escalating check should apply to these as well
ok
deads2k
force-pushed
the
prevent-escalating-resource-access
branch
from
945081e to
91545d2
Compare
|
@stevekuznetsov small. ptal. |
| // escalatingScopeResources are resources that are considered escalating for scope evaluation | ||
| var escalatingScopeResources = []unversioned.GroupResource{ | ||
| {Group: kapi.GroupName, Resource: "secrets"}, | ||
| /*imageapi.GroupName*/ {Group: "", Resource: "imagestreams/secrets"}, |
There was a problem hiding this comment.
I don't really understand why these are comments instead of the actual group names
There was a problem hiding this comment.
I don't really understand why these are comments instead of the actual group names
We don't want to introduce a dependency from this package to that one. Authorization should be a clean-ish layer, if possible.
There was a problem hiding this comment.
I don't really buy that, what do we gain from two fewer import statements?
There was a problem hiding this comment.
I don't really buy that, what do we gain from two fewer import statements?
Can be split out as a separate component. It doesn't care whether various api groups are present.
deads2k
force-pushed
the
prevent-escalating-resource-access
branch
from
91545d2 to
4a849d2
Compare
|
[merge] |
openshift-bot
added
the
needs-rebase
deads2k
force-pushed
the
prevent-escalating-resource-access
branch
from
4a849d2 to
25def50
Compare
|
Evaluated for origin merge up to 25def50 |
|
[Test]ing while waiting on the merge queue |
|
Evaluated for origin test up to 25def50 |
openshift-bot
removed
the
needs-rebase
|
@danmcp mean anything to you? |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/5891/) (Image: devenv-rhel7_4168) |
|
@deads2k Yes. It should be fixed shortly. |
|
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/3768/) |
This prevents scopes from having access to escalating resource by default, but also provides a means to request a role based scope that is escalating by adding
:!to the end of the "normal" definitionrole:<role name>:<effective namespace[:!].@sgallagher ptal I think this is the last bit required to unblock useful scopes.