Enforce authz on cost, policy and scanner report endpoints#427
Open
tamalsaha wants to merge 2 commits into
Open
Enforce authz on cost, policy and scanner report endpoints#427tamalsaha wants to merge 2 commits into
tamalsaha wants to merge 2 commits into
Conversation
The CostReport, PolicyReport and CVEReport (scanner) endpoints returned cluster data for a user-supplied scope without checking whether the caller was allowed to read it. Unlike the core genericresource/podview storages, none of them received an authorizer. Add an authorizer to each storage (wired in apiserver.go) and gate every request, with a small shared helper (pkg/shared/authz.go): - scanner CVEReport: authorize the pods that back the requested scope, the same access graph.LocatePods reads - cluster/image/cluster-CVE require cluster-wide "list pods", namespace scopes require "list pods" in that namespace, a pod request requires "get" on the pod, and an object request requires "get" on the referenced object. - policy PolicyReport: cluster scope requires cluster-wide read, namespace scope requires access to the namespace, and a resource scope requires "get" on the referenced object. - cost CostReport: aggregates whole-cluster spend with no single resource to scope to, so require cluster-wide read access. Cluster-wide read is modeled as get on */* (effectively cluster-admin), used only where the response exposes whole-cluster data. 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 using a fake authorizer (and a fake client with a static RESTMapper) that assert each report endpoint denies with a Forbidden status and authorizes against the expected attributes for every request scope: - shared.Authorize: allow, deny->Forbidden, missing-user->BadRequest, and ClusterReadAttributes shape. - scanner CVEReport: cluster/namespace/pod/object scopes map to the right pod/object access. - policy PolicyReport: cluster/namespace/resource scopes. - cost CostReport: requires cluster-wide read; missing user is a BadRequest. 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
CostReport(cost.k8s.appscode.com),PolicyReport(policy.k8s.appscode.com) and scannerCVEReportendpoints each took a user-supplied scope and returned cluster data without any authorization check. None of the three storages received an authorizer, unlike the coregenericresource/podviewstorages which already do per-object RBAC. As a result any caller able to reach these endpoints could read cluster-wide cost data, all gatekeeper policy violations, and CVE/image data for arbitrary pods — and the scanner endpoint additionally triggers image scans as a side effect.Fix
Add an
authorizer.Authorizerto each storage (wired inapiserver.go) and gate every request before any cluster data is read, using a small shared helperpkg/shared/authz.go(Authorize+ClusterReadAttributes).The required access matches what each endpoint actually reads:
CVEReport— mirrorsgraph.LocatePods:list podslist podsin that namespacegeton the podgeton the referenced objectPolicyReport:geton the referenced objectCostReport— aggregates whole-cluster spend with no single resource to scope to → cluster-wide read.Cluster-wide read is modeled as
geton*/*(effectively cluster-admin) and is used only where the response genuinely exposes whole-cluster data.Scope / notes
Follow-up to #425 (resourcegraph/renderrawgraph authz). With this, all of the registry endpoints that returned cluster data without a check are now gated.
render/resourcequeryare intentionally untouched — they already enforce access via the graph engine's user impersonation.Because cost and cluster-scope policy reports now require cluster-wide read, non-admin users who previously saw that data will get a 403; this is intentional given the data is whole-cluster, but worth confirming against the UI's expectations.
Testing
go build ./...passes.go vetandgofmtclean on the changed files.