multiple entries with the same spiffeID permission issue about spire-server OPA Engine

[loveyana]

I encountered a bug when doing spire-server OPA Engine verification.

When judging whether the entry is admin or downstream, spire-server used an entryFetcher to directly filter the specified entries in the database using spiffeID, and then any entry is The admin or downstream thinks that the request comes from an admin or downstream, which also leads to a problem:

If there are multiple entries with the same spiffeID, and one of them has the admin permission, the other entries with the same name also have the admin permission , In fact, I reproduced this problem during the test of k8s-workload-regsitrar, because by default the entries we created for regsitrar are /ns/spire/sa/k8s-workload-registrar, although we may be able to do it manually This problem is solved, but the spiffeID of the same name is actually allowed in the spire design, but it may not be easy to fix this problem.

I have not found a suitable way to get the accurate entry from the rpccontext to explain this problem, and If you have a solution, you can talk about it.

pkg/server/api/middleware/authorization_opa.go

func reconcileResult(ctx context.Context, res authpolicy.Result, entryFetcher EntryFetcher, agentAuthorizer AgentAuthorizer) (context.Context, bool, error) {
         ...
	// Get entries
	if res.AllowIfAdmin || res.AllowIfDownstream {
		ctx, entries, err := WithCallerEntries(ctx, entryFetcher)
		if err != nil {
			return ctx, false, err
		}
         ...
}

pkg/server/api/middleware/entries.go

// WithCallerEntries returns the caller entries retrieved using the given
// fetcher. If the context already has the caller entries, they are returned
// without re-fetching. This reduces entry fetching in the face of multiple
// authorizers.
func WithCallerEntries(ctx context.Context, entryFetcher EntryFetcher) (context.Context, []*types.Entry, error) {
	if entries, ok := ctx.Value(callerEntriesKey{}).([]*types.Entry); ok {
		return ctx, entries, nil
	}

	var entries []*types.Entry
	id, ok := rpccontext.CallerID(ctx)
	if !ok {
		return ctx, nil, nil
	}
	fmt.Println("CallerID is ", id)
        // Here we use spiffeID to get a entry list from ds
	entries, err := entryFetcher.FetchEntries(ctx, id)
	fmt.Println("CallerID is ", entries)
	if err != nil {
		rpccontext.Logger(ctx).WithError(err).Error("Failed to fetch caller entries")
		return nil, nil, status.Errorf(codes.Internal, "failed to fetch caller entries: %v", err)
	}
	return context.WithValue(ctx, callerEntriesKey{}, entries), entries, nil
}

pkg/server/endpoints/middleware.go

func EntryFetcher(ds datastore.DataStore) middleware.EntryFetcher {
	return middleware.EntryFetcherFunc(func(ctx context.Context, id spiffeid.ID) ([]*types.Entry, error) {
                 // Here is a ListRegistrationEntriesRequest Request
		resp, err := ds.ListRegistrationEntries(ctx, &datastore.ListRegistrationEntriesRequest{
			BySpiffeID: id.String(),
		})
		if err != nil {
			return nil, err
		}
		return api.RegistrationEntriesToProto(resp.Entries)
	})
}

• Version: c01d0b6
• Platform:
• Subsystem: server authorization_opa

[evan2645]

Hi @loveyana thank you for opening this issue. This is a known behavior in SPIRE, and I think that the OPA feature makes the situation quite a bit worse.

I have been thinking about it for a few days now, and chatted a bit with @azdagron. There are many potential solutions, including:

• Requiring all-or-nothing for the admin and downstream bits, e.g. preventing the creation of an entry with matching SPIFFE ID but without admin and downstream bits set
• Including a reference to the original entry (e.g. the entry ID) in SVIDs that SPIRE Server mints
• Recording all SVID issuances and the associated serial numbers

I think the last two are similar in that they strive to find a way to connect the caller's SVID back to an originating entry.

A separate but related problem in SPIRE has been the growth and scope of the entry object, which now includes information about three different "things": an identity, a workload, and authorization data. It's the last bit that's the subject of this issue.

It has always been the case that the privilege being granted by admin or downstream flag is being granted to the SPIFFE ID named in the entry, and not the workload named in the entry. For example, here is what the documentation says about the admin bit:

If set, the SPIFFE ID in this entry will be granted access to the Server APIs

When we think about SPIFFE and authorization, we generally think about policies that revolve around a SPIFFE ID. It just so happens that SPIRE is in a unique position, and can differentiate between workload instances that share the same identity... and I think this is the main differentiator between the options above.

All this considered, my vote is on the first option "all or nothing". If an entry exists that has admin or downstream bit set, all entries with the same SPIFFE ID being created after must also have it. Contrarily, if an existing entry does not have the admin bit set, then all subsequent entries created with the same SPIFFE ID must also not have it set. I think it is the least surprising (why doesn't my admin ID work as admin?) and the most well aligned with how non-SPIRE projects do SPIFFE authorization.

Note: the options above aren't technically mutually exclusive.

[loveyana]

@evan2645 I am glad that this issue has not been closed, I want to add another issue here.

I hope that the community can consider solving it before the OPA feature is officially put into use.

My team and I discovered another issue during the trial of the OPA feature, It is not a problem introduced by OPA, but it is a challenge to spire's flexible permission management.

When I used opa to control the entry permission of the k8s-workload-registrar component, I saw that it used the BatchDeleteEntry interface when in CRD mode, and it passed It is a list of entryIDs.

I don’t think I can properly manage the permissions of a cluster registry only through the OPA strategy. I think maybe some values should be converted in grpc requests, but this may bring too much overhead.

Hope to hear your opinion, thank you !

[rturner3]

When I used opa to control the entry permission of the k8s-workload-registrar component, I saw that it used the BatchDeleteEntry interface when in CRD mode, and it passed It is a list of entryIDs.

I don’t think I can properly manage the permissions of a cluster registry only through the OPA strategy. I think maybe some values should be converted in grpc requests, but this may bring too much overhead.

Sorry for the very delayed response, @loveyana ! I think there is definitely a gap here for managing authorization of entry update and delete by caller SPIFFE ID path namespace. For the create entry operation, we can come to an authorization decision using just the fields of the request. But for update and delete, we need to know the corresponding SPIFFE IDs of the entries being updated/deleted in order to determine whether the entries from the request have SPIFFE IDs in namespaces that are authorized for the caller. For the update operation specifically, the SPIFFE IDs will be in the request, but we cannot trust that information since they could be spoofed by the client (i.e. actual entry in the database corresponding to the request entry ID has a different SPIFFE ID than the one in the request). In both cases, we need to know the state of the entries from the database.

One approach that comes to mind is to introduce a new input field for the call to OPA of type interface{} that can be used flexibly by different methods to pass additional context to the Rego evaluation. These additional properties can be documented so that users can leverage them in their Rego policies. For select methods like BatchUpdateEntry and BatchDeleteEntry, we could enrich the Rego input by passing the SPIFFE IDs of the entries from the request.

The primary advantages I see for this approach are that we can maintain a unified authorization layer using OPA while retaining flexibility for future use cases.

The disadvantage with passing data from the database to OPA is that it may make the authorization cost more expensive since there are additional lookups required before the Rego can be evaluated. However, for the BatchUpdateEntry and BatchDeleteEntry cases, we may be able to avoid looking anything up from the database by extending the existing in-memory entry cache that is used in the Agent authorized entry fetch procedure.

OPA offers other ways of working with external data in Rego policies. Looking at each of them:

• JWT doesn't seem to make much sense here since we need to fetch from the database, which does not accept JWTs
• Bundle APIs seems highly undesirable since it requires bundling the data with the policy itself. Entries can be dynamic and there can be many entries, so keeping the DB in sync with the bundled policy seems infeasible. And hard to secure.
• Push data looks suboptimal for this use case since entry sets can be very large. We already cache all the entries in memory, so this would be a duplicate cache just for authorization.
• Pull data doesn't look like it will work easily because it can only pull data based on builtin Rego functions, which don't offer any way to directly connect to a SQL database. For this mode to work, there would probably need to be some dedicated local HTTP server that OPA can talk to.

Of the available options I see, it seems like enhancing the input to OPA is the most straightforward.

@evan2645 Any thoughts about this approach? I seem to remember you having some more ideas on this.

[rturner3]

Closing this in favor of #3620.