diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index 96c8807ce0564..6f8e805043fb7 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -5148,6 +5148,10 @@ func (a *ServerWithRoles) CreateKubernetesCluster(ctx context.Context, cluster t if err := a.checkAccessToKubeCluster(cluster); err != nil { return trace.Wrap(err) } + // Don't allow discovery service to create clusters with dynamic labels. + if a.hasBuiltinRole(types.RoleDiscovery) && len(cluster.GetDynamicLabels()) > 0 { + return trace.AccessDenied("discovered kubernetes cluster must not have dynamic labels") + } return trace.Wrap(a.authServer.CreateKubernetesCluster(ctx, cluster)) } @@ -5168,6 +5172,10 @@ func (a *ServerWithRoles) UpdateKubernetesCluster(ctx context.Context, cluster t if err := a.checkAccessToKubeCluster(cluster); err != nil { return trace.Wrap(err) } + // Don't allow discovery service to create clusters with dynamic labels. + if a.hasBuiltinRole(types.RoleDiscovery) && len(cluster.GetDynamicLabels()) > 0 { + return trace.AccessDenied("discovered kubernetes cluster must not have dynamic labels") + } return trace.Wrap(a.authServer.UpdateKubernetesCluster(ctx, cluster)) } @@ -5278,6 +5286,10 @@ func (a *ServerWithRoles) CreateDatabase(ctx context.Context, database types.Dat if err := a.checkAccessToDatabase(database); err != nil { return trace.Wrap(err) } + // Don't allow discovery service to create databases with dynamic labels. + if a.hasBuiltinRole(types.RoleDiscovery) && len(database.GetDynamicLabels()) > 0 { + return trace.AccessDenied("discovered database must not have dynamic labels") + } return trace.Wrap(a.authServer.CreateDatabase(ctx, database)) } @@ -5298,6 +5310,10 @@ func (a *ServerWithRoles) UpdateDatabase(ctx context.Context, database types.Dat if err := a.checkAccessToDatabase(database); err != nil { return trace.Wrap(err) } + // Don't allow discovery service to create databases with dynamic labels. + if a.hasBuiltinRole(types.RoleDiscovery) && len(database.GetDynamicLabels()) > 0 { + return trace.AccessDenied("discovered database must not have dynamic labels") + } return trace.Wrap(a.authServer.UpdateDatabase(ctx, database)) } diff --git a/lib/auth/auth_with_roles_test.go b/lib/auth/auth_with_roles_test.go index ee03c2575746f..fc4ded1b8d385 100644 --- a/lib/auth/auth_with_roles_test.go +++ b/lib/auth/auth_with_roles_test.go @@ -25,6 +25,8 @@ import ( "testing" "time" + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/eks" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/google/uuid" @@ -1679,18 +1681,148 @@ func TestDatabasesCRUDRBAC(t *testing.T) { // Dev should only be able to delete dev database. err = devClt.DeleteAllDatabases(ctx) require.NoError(t, err) - dbs, err = adminClt.GetDatabases(ctx) + mustGetDatabases(t, adminClt, []types.Database{adminDatabase}) + + // Admin should be able to delete all. + err = adminClt.DeleteAllDatabases(ctx) require.NoError(t, err) - require.Empty(t, cmp.Diff([]types.Database{adminDatabase}, dbs, + mustGetDatabases(t, adminClt, nil) + + t.Run("discovery service", func(t *testing.T) { + t.Cleanup(func() { + require.NoError(t, adminClt.DeleteAllDatabases(ctx)) + }) + + // Prepare discovery service client. + discoveryClt, err := srv.NewClient(TestBuiltin(types.RoleDiscovery)) + require.NoError(t, err) + + cloudDatabase, err := types.NewDatabaseV3(types.Metadata{ + Name: "cloud1", + Labels: map[string]string{"env": "prod", types.OriginLabel: types.OriginCloud}, + }, types.DatabaseSpecV3{ + Protocol: libdefaults.ProtocolMySQL, + URI: "localhost:3306", + }) + require.NoError(t, err) + + // Create a non-cloud database. + require.NoError(t, adminClt.CreateDatabase(ctx, adminDatabase)) + mustGetDatabases(t, adminClt, []types.Database{adminDatabase}) + + t.Run("cannot create non-cloud database", func(t *testing.T) { + require.True(t, trace.IsAccessDenied(discoveryClt.CreateDatabase(ctx, devDatabase))) + require.True(t, trace.IsAccessDenied(discoveryClt.UpdateDatabase(ctx, adminDatabase))) + }) + t.Run("cannot create database with dynamic labels", func(t *testing.T) { + cloudDatabaseWithDynamicLabels, err := types.NewDatabaseV3(types.Metadata{ + Name: "cloud2", + Labels: map[string]string{"env": "prod", types.OriginLabel: types.OriginCloud}, + }, types.DatabaseSpecV3{ + Protocol: libdefaults.ProtocolMySQL, + URI: "localhost:3306", + DynamicLabels: map[string]types.CommandLabelV2{ + "hostname": types.CommandLabelV2{ + Period: types.Duration(time.Hour), + Command: []string{"hostname"}, + }, + }, + }) + require.NoError(t, err) + require.True(t, trace.IsAccessDenied(discoveryClt.CreateDatabase(ctx, cloudDatabaseWithDynamicLabels))) + }) + t.Run("can create cloud database", func(t *testing.T) { + require.NoError(t, discoveryClt.CreateDatabase(ctx, cloudDatabase)) + require.NoError(t, discoveryClt.UpdateDatabase(ctx, cloudDatabase)) + }) + t.Run("can get only cloud database", func(t *testing.T) { + mustGetDatabases(t, discoveryClt, []types.Database{cloudDatabase}) + }) + t.Run("can delete only cloud database", func(t *testing.T) { + require.NoError(t, discoveryClt.DeleteAllDatabases(ctx)) + mustGetDatabases(t, discoveryClt, nil) + mustGetDatabases(t, adminClt, []types.Database{adminDatabase}) + }) + }) +} + +func mustGetDatabases(t *testing.T, client *Client, wantDatabases []types.Database) { + t.Helper() + + actualDatabases, err := client.GetDatabases(context.Background()) + require.NoError(t, err) + + require.Empty(t, cmp.Diff(wantDatabases, actualDatabases, cmpopts.IgnoreFields(types.Metadata{}, "ID"), + cmpopts.EquateEmpty(), )) +} - // Admin should be able to delete all. - err = adminClt.DeleteAllDatabases(ctx) +func TestKubernetesClusterCRUD_DiscoveryService(t *testing.T) { + t.Parallel() + ctx := context.Background() + srv := newTestTLSServer(t) + + discoveryClt, err := srv.NewClient(TestBuiltin(types.RoleDiscovery)) require.NoError(t, err) - dbs, err = adminClt.GetDatabases(ctx) + + eksCluster, err := services.NewKubeClusterFromAWSEKS(&eks.Cluster{ + Name: aws.String("eks-cluster1"), + Arn: aws.String("arn:aws:eks:eu-west-1:accountID:cluster/cluster1"), + Status: aws.String(eks.ClusterStatusActive), + }) + require.NoError(t, err) + + // Discovery service must not have access to non-cloud cluster (cluster + // without "cloud" origin label). + nonCloudCluster, err := types.NewKubernetesClusterV3( + types.Metadata{ + Name: "non-cloud", + }, + types.KubernetesClusterSpecV3{}, + ) require.NoError(t, err) - require.Len(t, dbs, 0) + require.NoError(t, srv.Auth().CreateKubernetesCluster(ctx, nonCloudCluster)) + + // Discovery service cannot create cluster with dynamic labels. + clusterWithDynamicLabels, err := services.NewKubeClusterFromAWSEKS(&eks.Cluster{ + Name: aws.String("eks-cluster2"), + Arn: aws.String("arn:aws:eks:eu-west-1:accountID:cluster/cluster2"), + Status: aws.String(eks.ClusterStatusActive), + }) + require.NoError(t, err) + clusterWithDynamicLabels.SetDynamicLabels(map[string]types.CommandLabel{ + "hostname": &types.CommandLabelV2{ + Period: types.Duration(time.Hour), + Command: []string{"hostname"}, + }, + }) + + t.Run("Create", func(t *testing.T) { + require.NoError(t, discoveryClt.CreateKubernetesCluster(ctx, eksCluster)) + require.True(t, trace.IsAccessDenied(discoveryClt.CreateKubernetesCluster(ctx, nonCloudCluster))) + require.True(t, trace.IsAccessDenied(discoveryClt.CreateKubernetesCluster(ctx, clusterWithDynamicLabels))) + }) + t.Run("Read", func(t *testing.T) { + clusters, err := discoveryClt.GetKubernetesClusters(ctx) + require.NoError(t, err) + require.Equal(t, clusters, []types.KubeCluster{eksCluster}) + }) + t.Run("Update", func(t *testing.T) { + require.NoError(t, discoveryClt.UpdateKubernetesCluster(ctx, eksCluster)) + require.True(t, trace.IsAccessDenied(discoveryClt.UpdateKubernetesCluster(ctx, nonCloudCluster))) + }) + t.Run("Delete", func(t *testing.T) { + require.NoError(t, discoveryClt.DeleteAllKubernetesClusters(ctx)) + clusters, err := discoveryClt.GetKubernetesClusters(ctx) + require.NoError(t, err) + require.Empty(t, clusters) + + // Discovery service cannot delete non-cloud clusters. + clusters, err = srv.Auth().GetKubernetesClusters(ctx) + require.NoError(t, err) + require.Len(t, clusters, 1) + }) } func TestGetAndList_DatabaseServers(t *testing.T) { diff --git a/lib/auth/grpcserver.go b/lib/auth/grpcserver.go index ee0ee4f67d535..02c63f0d99c58 100644 --- a/lib/auth/grpcserver.go +++ b/lib/auth/grpcserver.go @@ -4750,7 +4750,10 @@ func (g *GRPCServer) UpdateKubernetesCluster(ctx context.Context, cluster *types if err != nil { return nil, trace.Wrap(err) } - cluster.SetOrigin(types.OriginDynamic) + // if origin is not set, force it to be dynamic. + if len(cluster.Origin()) == 0 { + cluster.SetOrigin(types.OriginDynamic) + } if err := auth.UpdateKubernetesCluster(ctx, cluster); err != nil { return nil, trace.Wrap(err) } diff --git a/lib/authz/permissions.go b/lib/authz/permissions.go index e0dfb65aa6bee..ec7d43b487d92 100644 --- a/lib/authz/permissions.go +++ b/lib/authz/permissions.go @@ -833,9 +833,9 @@ func definitionForBuiltinRole(clusterName string, recConfig types.SessionRecordi types.NewRule(types.KindKubernetesCluster, services.RW()), types.NewRule(types.KindDatabase, services.RW()), }, - // wildcard any cluster available. - KubernetesLabels: types.Labels{types.Wildcard: []string{types.Wildcard}}, - DatabaseLabels: types.Labels{types.Wildcard: []string{types.Wildcard}}, + // Discovery service should only access kubes/dbs with "cloud" origin. + KubernetesLabels: types.Labels{types.OriginLabel: []string{types.OriginCloud}}, + DatabaseLabels: types.Labels{types.OriginLabel: []string{types.OriginCloud}}, }, }) case types.RoleOkta: diff --git a/lib/srv/discovery/discovery_test.go b/lib/srv/discovery/discovery_test.go index bd044a9e02f60..352c9a8ef4201 100644 --- a/lib/srv/discovery/discovery_test.go +++ b/lib/srv/discovery/discovery_test.go @@ -1124,7 +1124,11 @@ func TestDiscoveryDatabase(t *testing.T) { select { case <-waitForReconcile: - actualDatabases, err := authClient.GetDatabases(ctx) + // Use tlsServer.Auth() instead of authClient to compare + // databases stored in auth. authClient was created with + // types.RoleDiscovery and it does not have permissions to + // access non-cloud databases. + actualDatabases, err := tlsServer.Auth().GetDatabases(ctx) require.NoError(t, err) require.Empty(t, cmp.Diff(tc.expectDatabases, actualDatabases, cmpopts.IgnoreFields(types.Metadata{}, "ID"),