Enforce authz on resourcegraph and renderrawgraph endpoints#425
Open
tamalsaha wants to merge 2 commits into
Open
Enforce authz on resourcegraph and renderrawgraph endpoints#425tamalsaha wants to merge 2 commits into
tamalsaha wants to merge 2 commits into
Conversation
The ResourceGraph and RenderRawGraph endpoints returned the object graph for an arbitrary user-supplied source resource without checking whether the caller was allowed to read it. The graph response exposes the names and namespaces of related objects across the cluster, so any user able to call the endpoint could enumerate cluster topology for resources they otherwise cannot see. Gate both endpoints with an RBAC check, mirroring the per-object authorization already done by the core genericresource/podview storages: - When a source object is supplied, require "get" access to it before returning its graph. - RenderRawGraph additionally renders the whole-cluster graph when no source is given; that path now requires cluster-wide read access (get on */*). The rbacAuthorizer is now passed to both storages in apiserver.go. Signed-off-by: Tamal Saha <tamal@appscode.com>
![kodiakhq[bot]](https://avatars.githubusercontent.com/in/29196?s=60&v=4){kind=small}
Add unit tests with a fake authorizer (and a fake client with a static RESTMapper) asserting: - resourcegraph: a source-scoped request denies with Forbidden and authorizes "get" on the referenced object; missing user info is a BadRequest. - renderrawgraph: a source-scoped request authorizes "get" on the object, while the no-source whole-cluster graph requires cluster-wide read (get */*). Signed-off-by: Tamal Saha <tamal@appscode.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The
ResourceGraph(meta.k8s.appscode.com/resourcegraphs) andRenderRawGraph(renderrawgraphs) endpoints built and returned the object graph for an arbitrary, user-supplied source resource without any authorization check on that resource. Unlike the coregenericresource,podview, andresourceservicestorages — which already perform per-object RBAC checks — these two received no authorizer at all.Because the graph response includes the names and namespaces of parent/child/related objects discovered cluster-wide, any caller able to reach the endpoint could enumerate cluster topology for resources they otherwise cannot read.
Fix
Gate both endpoints with the same RBAC pattern used elsewhere in the registry:
rbacAuthorizeris now passed to both storages inapiserver.go.getaccess to it before the graph is returned.RenderRawGraphalso renders the whole-cluster graph when no source is given; that path now requires cluster-wide read access (geton*/*, i.e. effectively cluster-admin).For the UI's normal use the caller is already viewing the source object (so already has
geton it), so legitimate requests are unaffected; the change only closes access for resources the caller cannot read.Notes / scope
This is the first of several authorization fixes identified in a codebase review. It covers the two endpoints with unambiguous "get on the source object" semantics. Other endpoints (
scanner/reports, cluster-scopecost/reportsandpolicy/reports) need separate, more careful handling and are intentionally not included here. Therender/resourcequeryendpoints already enforce authorization via the graph engine's user impersonation, so they are out of scope too.Testing
go build ./...passes.go vetandgofmtclean on the changed files.