Skip to content

Add federation routes to RBAC static API list - #747

Open
arpitjain099 wants to merge 1 commit into
spiffe:mainfrom
arpitjain099:fix/rbac-federation-routes
Open

Add federation routes to RBAC static API list#747
arpitjain099 wants to merge 1 commit into
spiffe:mainfrom
arpitjain099:fix/rbac-federation-routes

Conversation

@arpitjain099

Copy link
Copy Markdown

The federation management routes are registered on the API router in api/agent/server.go but never made it into staticAPIV1List in pkg/agent/authorization/rbac.go:

  • /api/v1/spire/federations GET/POST/PATCH/DELETE (server.go:210-213)
  • /api/v1/spire-controller-manager/clusterfederatedtrustdomains GET/POST (server.go:216-217)

Because the list is what validateInitParameters checks policies against, naming either path in an RBAC policy fails startup with "API V1 path ... does not exist with method ...". And since they can't be in the mapping, authorizeAPIV1Request finds no allowed roles and denies them for everyone. So with RBAC enabled these endpoints are unreachable regardless of role, even for admin.

The direction is fail-closed, so this isn't an authorization bypass, just federation management being unusable under RBAC.

The sibling /api/v1/spire/federations/bundles entry already covers all four of its methods, so I followed that entry's shape. Methods match what the router registers. OPTIONS is left out on purpose since verificationMiddleware short-circuits preflight before authorization, same as every other entry.

Test: extended rbac_test.go with TestRBACPolicyAcceptsFederationRoutes, which builds a one-route policy for each path/method pair and asserts NewRBACAuthorizer accepts it.

Before the fix, all six pairs fail:

--- FAIL: TestRBACPolicyAcceptsFederationRoutes (0.00s)
    rbac_test.go:97: ERROR: policy naming GET /api/v1/spire/federations rejected: Could not parse policy testPolicy: invalid mapping: API V1 path /api/v1/spire/federations does not exist with method GET
    rbac_test.go:97: ERROR: policy naming POST /api/v1/spire/federations rejected: ... does not exist with method POST
    rbac_test.go:97: ERROR: policy naming PATCH /api/v1/spire/federations rejected: ... does not exist with method PATCH
    rbac_test.go:97: ERROR: policy naming DELETE /api/v1/spire/federations rejected: ... does not exist with method DELETE
    rbac_test.go:97: ERROR: policy naming GET /api/v1/spire-controller-manager/clusterfederatedtrustdomains rejected: ... does not exist with method GET
    rbac_test.go:97: ERROR: policy naming POST /api/v1/spire-controller-manager/clusterfederatedtrustdomains rejected: ... does not exist with method POST
FAIL

After:

=== RUN   TestNewRBACAuthorizer
--- PASS: TestNewRBACAuthorizer (0.00s)
=== RUN   TestRBACPolicyAcceptsFederationRoutes
--- PASS: TestRBACPolicyAcceptsFederationRoutes (0.00s)
PASS
ok  	github.com/spiffe/tornjak/pkg/agent/authorization	0.431s

go build ./... is clean. The existing TestNewRBACAuthorizer still passes, so the added entries don't loosen the negative cases.

I work on supply-chain security and was reading through Tornjak's authorization layer when I noticed the route table and the static list had drifted apart.

The federation management routes are registered and served in
api/agent/server.go but are absent from staticAPIV1List, so
validateInitParameters rejects any RBAC policy that names them and
authorizeAPIV1Request denies them for every role. That leaves
/api/v1/spire/federations and the clusterfederatedtrustdomains route
unreachable whenever RBAC is enabled.

Adds both to the static list with the methods the router registers,
matching the existing /api/v1/spire/federations/bundles entry.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant