luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

grpc/acl: relax permissions required for "core" endpoints (#15346) 

Previously, these endpoints required `service:write` permission on _any_
service as a sort of proxy for "is the caller allowed to participate in
the mesh?".

Now, they're called as part of the process of establishing a server
connection by any consumer of the consul-server-connection-manager
library, which will include non-mesh workloads (e.g. Consul KV as a
storage backend for Vault) as well as ancillary components such as
consul-k8s' acl-init process, which likely won't have `service:write`
permission.

So this commit relaxes those requirements to accept *any* valid ACL token
on the following gRPC endpoints:

- `hashicorp.consul.dataplane.DataplaneService/GetSupportedDataplaneFeatures`
- `hashicorp.consul.serverdiscovery.ServerDiscoveryService/WatchServers`
- `hashicorp.consul.connectca.ConnectCAService/WatchRoots`
This commit is contained in:
Dan Upton 2023-01-04 12:40:34 +00:00 committed by GitHub
parent af2464126a
commit 4719b717ea
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 126 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.changelog/15346.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:enhancement
acl: relax permissions on the `WatchServers`, `WatchRoots` and `GetSupportedDataplaneFeatures` gRPC endpoints to accept *any* valid ACL token
```

14
agent/grpc-external/services/connectca/sign_test.go
View File

@ -33,7 +33,7 @@ func TestSign_ConnectDisabled(t *testing.T) {
func TestSign_Validation(t *testing.T) {
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything).
Return(testutils.TestAuthorizerAllowAll(t), nil)
Return(testutils.ACLAllowAll(t), nil)
server := NewServer(Config{
Logger: hclog.NewNullLogger(),
@ -90,7 +90,7 @@ func TestSign_Unauthenticated(t *testing.T) {
func TestSign_PermissionDenied(t *testing.T) {
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything).
Return(testutils.TestAuthorizerAllowAll(t), nil)
Return(testutils.ACLAllowAll(t), nil)
caManager := &MockCAManager{}
caManager.On("AuthorizeAndSignCertificate", mock.Anything, mock.Anything).
@ -116,7 +116,7 @@ func TestSign_PermissionDenied(t *testing.T) {
func TestSign_InvalidCSR(t *testing.T) {
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything).
Return(testutils.TestAuthorizerAllowAll(t), nil)
Return(testutils.ACLAllowAll(t), nil)
caManager := &MockCAManager{}
caManager.On("AuthorizeAndSignCertificate", mock.Anything, mock.Anything).
@ -142,7 +142,7 @@ func TestSign_InvalidCSR(t *testing.T) {
func TestSign_RateLimited(t *testing.T) {
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything).
Return(testutils.TestAuthorizerAllowAll(t), nil)
Return(testutils.ACLAllowAll(t), nil)
caManager := &MockCAManager{}
caManager.On("AuthorizeAndSignCertificate", mock.Anything, mock.Anything).
@ -168,7 +168,7 @@ func TestSign_RateLimited(t *testing.T) {
func TestSign_InternalError(t *testing.T) {
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything).
Return(testutils.TestAuthorizerAllowAll(t), nil)
Return(testutils.ACLAllowAll(t), nil)
caManager := &MockCAManager{}
caManager.On("AuthorizeAndSignCertificate", mock.Anything, mock.Anything).
@ -194,7 +194,7 @@ func TestSign_InternalError(t *testing.T) {
func TestSign_Success(t *testing.T) {
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything).
Return(testutils.TestAuthorizerAllowAll(t), nil)
Return(testutils.ACLAllowAll(t), nil)
caManager := &MockCAManager{}
caManager.On("AuthorizeAndSignCertificate", mock.Anything, mock.Anything).
@ -220,7 +220,7 @@ func TestSign_Success(t *testing.T) {
func TestSign_RPCForwarding(t *testing.T) {
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything).
Return(testutils.TestAuthorizerAllowAll(t), nil)
Return(testutils.ACLAllowAll(t), nil)
caManager := &MockCAManager{}
caManager.On("AuthorizeAndSignCertificate", mock.Anything, mock.Anything).

2
agent/grpc-external/services/connectca/watch_roots.go
View File

@ -58,7 +58,7 @@ func (s *Server) serveRoots(
serverStream pbconnectca.ConnectCAService_WatchRootsServer,
logger hclog.Logger,
) (uint64, error) {
if err := s.authorize(token); err != nil {
if err := external.RequireAnyValidACLToken(s.ACLResolver, token); err != nil {
return 0, err
}

12
agent/grpc-external/services/connectca/watch_roots_test.go
View File

@ -54,7 +54,7 @@ func TestWatchRoots_Success(t *testing.T) {
// Mock the ACL Resolver to return an authorizer with `service:write`.
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", testACLToken, mock.Anything, mock.Anything).
Return(testutils.TestAuthorizerServiceWriteAny(t), nil)
Return(testutils.ACLNoPermissions(t), nil)
options := structs.QueryOptions{Token: testACLToken}
ctx, err := external.ContextWithQueryOptions(context.Background(), options)
@ -144,7 +144,7 @@ func TestWatchRoots_ACLTokenInvalidated(t *testing.T) {
// first two times it is called (initial connect and first re-auth).
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", testACLToken, mock.Anything, mock.Anything).
Return(testutils.TestAuthorizerServiceWriteAny(t), nil).Twice()
Return(testutils.ACLNoPermissions(t), nil).Twice()
options := structs.QueryOptions{Token: testACLToken}
ctx, err := external.ContextWithQueryOptions(context.Background(), options)
@ -184,9 +184,9 @@ func TestWatchRoots_ACLTokenInvalidated(t *testing.T) {
// Expect the stream to remain open and to receive the new roots.
mustGetRoots(t, rspCh)
// Simulate removing the `service:write` permission.
// Simulate deleting the ACL token.
aclResolver.On("ResolveTokenAndDefaultMeta", testACLToken, mock.Anything, mock.Anything).
Return(testutils.TestAuthorizerDenyAll(t), nil)
Return(resolver.Result{}, acl.ErrNotFound)
// Update the ACL token to cause the subscription to be force-closed.
err = fsm.GetStore().ACLTokenSet(1, &structs.ACLToken{
@ -197,7 +197,7 @@ func TestWatchRoots_ACLTokenInvalidated(t *testing.T) {
// Expect the stream to be terminated.
err = mustGetError(t, rspCh)
require.Equal(t, codes.PermissionDenied.String(), status.Code(err).String())
require.Equal(t, codes.Unauthenticated.String(), status.Code(err).String())
}
func TestWatchRoots_StateStoreAbandoned(t *testing.T) {
@ -214,7 +214,7 @@ func TestWatchRoots_StateStoreAbandoned(t *testing.T) {
// Mock the ACL Resolver to return an authorizer with `service:write`.
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", testACLToken, mock.Anything, mock.Anything).
Return(testutils.TestAuthorizerServiceWriteAny(t), nil)
Return(testutils.ACLNoPermissions(t), nil)
options := structs.QueryOptions{Token: testACLToken}
ctx, err := external.ContextWithQueryOptions(context.Background(), options)

6
agent/grpc-external/services/dataplane/get_envoy_bootstrap_params_test.go
View File

@ -126,7 +126,7 @@ func TestGetEnvoyBootstrapParams_Success(t *testing.T) {
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", testToken, mock.Anything, mock.Anything).
Return(testutils.TestAuthorizerServiceRead(t, tc.registerReq.Service.ID), nil)
Return(testutils.ACLServiceRead(t, tc.registerReq.Service.ID), nil)
options := structs.QueryOptions{Token: testToken}
ctx, err := external.ContextWithQueryOptions(context.Background(), options)
@ -233,7 +233,7 @@ func TestGetEnvoyBootstrapParams_Error(t *testing.T) {
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", testToken, mock.Anything, mock.Anything).
Return(testutils.TestAuthorizerServiceRead(t, proxyServiceID), nil)
Return(testutils.ACLServiceRead(t, proxyServiceID), nil)
options := structs.QueryOptions{Token: testToken}
ctx, err := external.ContextWithQueryOptions(context.Background(), options)
@ -329,7 +329,7 @@ func TestGetEnvoyBootstrapParams_PermissionDenied(t *testing.T) {
// Mock the ACL resolver to return a deny all authorizer
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", testToken, mock.Anything, mock.Anything).
Return(testutils.TestAuthorizerDenyAll(t), nil)
Return(testutils.ACLNoPermissions(t), nil)
options := structs.QueryOptions{Token: testToken}
ctx, err := external.ContextWithQueryOptions(context.Background(), options)

14
agent/grpc-external/services/dataplane/get_supported_features.go
View File

@ -6,9 +6,7 @@ import (
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
acl "github.com/hashicorp/consul/acl"
external "github.com/hashicorp/consul/agent/grpc-external"
structs "github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/proto-public/pbdataplane"
)
@ -18,20 +16,12 @@ func (s *Server) GetSupportedDataplaneFeatures(ctx context.Context, req *pbdatap
logger.Trace("Started processing request")
defer logger.Trace("Finished processing request")
// Require the given ACL token to have `service:write` on any service
options, err := external.QueryOptionsFromContext(ctx)
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}
var authzContext acl.AuthorizerContext
entMeta := structs.WildcardEnterpriseMetaInPartition(structs.WildcardSpecifier)
authz, err := s.ACLResolver.ResolveTokenAndDefaultMeta(options.Token, entMeta, &authzContext)
if err != nil {
return nil, status.Error(codes.Unauthenticated, err.Error())
}
if err := authz.ToAllowAuthorizer().ServiceWriteAnyAllowed(&authzContext); err != nil {
return nil, status.Error(codes.PermissionDenied, err.Error())
if err := external.RequireAnyValidACLToken(s.ACLResolver, options.Token); err != nil {
return nil, err
}
supportedFeatures := []*pbdataplane.DataplaneFeatureSupport{

33
agent/grpc-external/services/dataplane/get_supported_features_test.go
View File

@ -24,7 +24,7 @@ func TestSupportedDataplaneFeatures_Success(t *testing.T) {
// Mock the ACL Resolver to return an authorizer with `service:write`.
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", testACLToken, mock.Anything, mock.Anything).
Return(testutils.TestAuthorizerServiceWriteAny(t), nil)
Return(testutils.ACLServiceWriteAny(t), nil)
options := structs.QueryOptions{Token: testACLToken}
ctx, err := external.ContextWithQueryOptions(context.Background(), options)
@ -53,7 +53,7 @@ func TestSupportedDataplaneFeatures_Success(t *testing.T) {
}
}
func TestSupportedDataplaneFeatures_Unauthenticated(t *testing.T) {
func TestSupportedDataplaneFeatures_InvalidACLToken(t *testing.T) {
// Mock the ACL resolver to return ErrNotFound.
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything).
@ -74,11 +74,11 @@ func TestSupportedDataplaneFeatures_Unauthenticated(t *testing.T) {
require.Nil(t, resp)
}
func TestSupportedDataplaneFeatures_PermissionDenied(t *testing.T) {
// Mock the ACL resolver to return a deny all authorizer
func TestSupportedDataplaneFeatures_AnonymousACLToken(t *testing.T) {
// Mock the ACL resolver to return ErrNotFound.
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", testACLToken, mock.Anything, mock.Anything).
Return(testutils.TestAuthorizerDenyAll(t), nil)
aclResolver.On("ResolveTokenAndDefaultMeta", mock.Anything, mock.Anything, mock.Anything).
Return(testutils.ACLAnonymous(t), nil)
options := structs.QueryOptions{Token: testACLToken}
ctx, err := external.ContextWithQueryOptions(context.Background(), options)
@ -91,6 +91,25 @@ func TestSupportedDataplaneFeatures_PermissionDenied(t *testing.T) {
client := testClient(t, server)
resp, err := client.GetSupportedDataplaneFeatures(ctx, &pbdataplane.GetSupportedDataplaneFeaturesRequest{})
require.Error(t, err)
require.Equal(t, codes.PermissionDenied.String(), status.Code(err).String())
require.Equal(t, codes.Unauthenticated.String(), status.Code(err).String())
require.Nil(t, resp)
}
func TestSupportedDataplaneFeatures_NoPermissions(t *testing.T) {
// Mock the ACL resolver to return a deny all authorizer
aclResolver := &MockACLResolver{}
aclResolver.On("ResolveTokenAndDefaultMeta", testACLToken, mock.Anything, mock.Anything).
Return(testutils.ACLNoPermissions(t), nil)
options := structs.QueryOptions{Token: testACLToken}
ctx, err := external.ContextWithQueryOptions(context.Background(), options)
require.NoError(t, err)
server := NewServer(Config{
Logger: hclog.NewNullLogger(),
ACLResolver: aclResolver,
})
client := testClient(t, server)
_, err = client.GetSupportedDataplaneFeatures(ctx, &pbdataplane.GetSupportedDataplaneFeaturesRequest{})
require.NoError(t, err)
}

18
agent/grpc-external/services/serverdiscovery/watch_servers.go
View File

@ -8,7 +8,6 @@ import (
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/agent/consul/autopilotevents"
"github.com/hashicorp/consul/agent/consul/stream"
external "github.com/hashicorp/consul/agent/grpc-external"
@ -46,7 +45,7 @@ func (s *Server) WatchServers(req *pbserverdiscovery.WatchServersRequest, server
}
func (s *Server) serveReadyServers(token string, index uint64, req *pbserverdiscovery.WatchServersRequest, serverStream pbserverdiscovery.ServerDiscoveryService_WatchServersServer, logger hclog.Logger) (uint64, error) {
if err := s.authorize(token); err != nil {
if err := external.RequireAnyValidACLToken(s.ACLResolver, token); err != nil {
return 0, err
}
@ -105,21 +104,6 @@ func (s *Server) serveReadyServers(token string, index uint64, req *pbserverdisc
}
}
func (s *Server) authorize(token string) error {
// Require the given ACL token to have `service:write` on any service (in any
// partition and namespace).
var authzContext acl.AuthorizerContext
entMeta := structs.WildcardEnterpriseMetaInPartition(structs.WildcardSpecifier)
authz, err := s.ACLResolver.ResolveTokenAndDefaultMeta(token, entMeta, &authzContext)
if err != nil {
return status.Error(codes.Unauthenticated, err.Error())
}
if err := authz.ToAllowAuthorizer().ServiceWriteAnyAllowed(&authzContext); err != nil {
return status.Error(codes.PermissionDenied, err.Error())
}
return nil
}
func eventToResponse(req *pbserverdiscovery.WatchServersRequest, event stream.Event) (*pbserverdiscovery.WatchServersResponse, error) {
readyServers, err := autopilotevents.ExtractEventPayload(event)
if err != nil {

8
agent/grpc-external/services/serverdiscovery/watch_servers_test.go
View File

@ -123,7 +123,7 @@ func TestWatchServers_StreamLifeCycle(t *testing.T) {
// 2 times authorization should succeed and the third should fail.
resolver := newMockACLResolver(t)
resolver.On("ResolveTokenAndDefaultMeta", testACLToken, mock.Anything, mock.Anything).
Return(testutils.TestAuthorizerServiceWriteAny(t), nil).Twice()
Return(testutils.ACLNoPermissions(t), nil).Twice()
// add the token to the requests context
options := structs.QueryOptions{Token: testACLToken}
@ -192,13 +192,13 @@ func TestWatchServers_StreamLifeCycle(t *testing.T) {
prototest.AssertDeepEqual(t, threeServerResponse, rsp)
}
func TestWatchServers_ACLToken_PermissionDenied(t *testing.T) {
func TestWatchServers_ACLToken_AnonymousToken(t *testing.T) {
// setup the event publisher and snapshot handler
_, publisher := setupPublisher(t)
resolver := newMockACLResolver(t)
resolver.On("ResolveTokenAndDefaultMeta", testACLToken, mock.Anything, mock.Anything).
Return(testutils.TestAuthorizerDenyAll(t), nil).Once()
Return(testutils.ACLAnonymous(t), nil).Once()
// add the token to the requests context
options := structs.QueryOptions{Token: testACLToken}
@ -222,7 +222,7 @@ func TestWatchServers_ACLToken_PermissionDenied(t *testing.T) {
// Expect to get an Unauthenticated error immediately.
err = mustGetError(t, rspCh)
require.Equal(t, codes.PermissionDenied.String(), status.Code(err).String())
require.Equal(t, codes.Unauthenticated.String(), status.Code(err).String())
}
func TestWatchServers_ACLToken_Unauthenticated(t *testing.T) {

49
agent/grpc-external/testutils/acl.go
View File

@ -5,23 +5,43 @@ import (
"github.com/stretchr/testify/require"
"github.com/hashicorp/go-uuid"
"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/acl/resolver"
"github.com/hashicorp/consul/agent/structs"
)
func TestAuthorizerAllowAll(t *testing.T) resolver.Result {
func ACLAnonymous(t *testing.T) resolver.Result {
t.Helper()
return resolver.Result{Authorizer: acl.AllowAll()}
return resolver.Result{
Authorizer: acl.DenyAll(),
ACLIdentity: &structs.ACLToken{
AccessorID: structs.ACLTokenAnonymousID,
},
}
}
func TestAuthorizerDenyAll(t *testing.T) resolver.Result {
func ACLAllowAll(t *testing.T) resolver.Result {
t.Helper()
return resolver.Result{Authorizer: acl.DenyAll()}
return resolver.Result{
Authorizer: acl.AllowAll(),
ACLIdentity: randomACLIdentity(t),
}
}
func TestAuthorizerServiceWriteAny(t *testing.T) resolver.Result {
func ACLNoPermissions(t *testing.T) resolver.Result {
t.Helper()
return resolver.Result{
Authorizer: acl.DenyAll(),
ACLIdentity: randomACLIdentity(t),
}
}
func ACLServiceWriteAny(t *testing.T) resolver.Result {
t.Helper()
policy, err := acl.NewPolicyFromSource(`
@ -34,10 +54,13 @@ func TestAuthorizerServiceWriteAny(t *testing.T) resolver.Result {
authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{policy}, nil)
require.NoError(t, err)
return resolver.Result{Authorizer: authz}
return resolver.Result{
Authorizer: authz,
ACLIdentity: randomACLIdentity(t),
}
}
func TestAuthorizerServiceRead(t *testing.T, serviceName string) resolver.Result {
func ACLServiceRead(t *testing.T, serviceName string) resolver.Result {
t.Helper()
aclRule := &acl.Policy{
@ -53,5 +76,15 @@ func TestAuthorizerServiceRead(t *testing.T, serviceName string) resolver.Result
authz, err := acl.NewPolicyAuthorizerWithDefaults(acl.DenyAll(), []*acl.Policy{aclRule}, nil)
require.NoError(t, err)
return resolver.Result{Authorizer: authz}
return resolver.Result{
Authorizer: authz,
ACLIdentity: randomACLIdentity(t),
}
}
func randomACLIdentity(t *testing.T) structs.ACLIdentity {
id, err := uuid.GenerateUUID()
require.NoError(t, err)
return &structs.ACLToken{AccessorID: id}
}

33
agent/grpc-external/utils.go
View File

@ -1,6 +1,14 @@
package external
import "github.com/hashicorp/go-uuid"
import (
"github.com/hashicorp/go-uuid"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/acl/resolver"
"github.com/hashicorp/consul/agent/structs"
)
// We tag logs with a unique identifier to ease debugging. In the future this
// should probably be a real Open Telemetry trace ID.
@ -11,3 +19,26 @@ func TraceID() string {
}
return id
}
type ACLResolver interface {
ResolveTokenAndDefaultMeta(string, *acl.EnterpriseMeta, *acl.AuthorizerContext) (resolver.Result, error)
}
// RequireAnyValidACLToken checks that the caller provided a valid ACL token
// without requiring any specific permissions. This is useful for endpoints
// that are used by all/most consumers of our API, such as those called by the
// consul-server-connection-manager library when establishing a new connection.
//
// Note: no token is required if ACLs are disabled.
func RequireAnyValidACLToken(resolver ACLResolver, token string) error {
authz, err := resolver.ResolveTokenAndDefaultMeta(token, nil, nil)
if err != nil {
return status.Error(codes.Unauthenticated, err.Error())
}
if id := authz.ACLIdentity; id == nil || id.ID() == structs.ACLTokenAnonymousID {
return status.Error(codes.Unauthenticated, "An ACL token must be provided (via the `x-consul-token` metadata field) to call this endpoint")
}
return nil
}