Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v15] fix: prevent deleting AWS OIDC integration used by EAS #40851

Merged
merged 3 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions api/proto/teleport/integration/v1/integration_service.proto
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ service IntegrationService {
rpc DeleteIntegration(DeleteIntegrationRequest) returns (google.protobuf.Empty);

// DeleteAllIntegrations removes all Integrations.
// DEPRECATED: Can't delete all integrations over gRPC.
rpc DeleteAllIntegrations(DeleteAllIntegrationsRequest) returns (google.protobuf.Empty);

// GenerateAWSOIDCToken generates a token to be used when executing an AWS OIDC Integration action.
Expand Down Expand Up @@ -88,6 +89,7 @@ message DeleteIntegrationRequest {
}

// DeleteAllIntegrationsRequest is the request for deleting all integrations.
// DEPRECATED: Can't delete all integrations over gRPC.
message DeleteAllIntegrationsRequest {}

// GenerateAWSOIDCTokenRequest are the parameters used to request an AWS OIDC
Expand Down
9 changes: 7 additions & 2 deletions api/types/externalauditstorage/externalauditstorage.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,14 @@ func (a *ExternalAuditStorage) MatchSearch(values []string) bool {
return types.MatchSearch(fieldVals, values, nil)
}

// CloneResource returns a copy of the resource as types.ResourceWithLabels.
func (a *ExternalAuditStorage) CloneResource() types.ResourceWithLabels {
// Clone returs a copy of the resource.
func (a *ExternalAuditStorage) Clone() *ExternalAuditStorage {
var copy *ExternalAuditStorage
utils.StrictObjectToStruct(a, &copy)
return copy
}

// CloneResource returns a copy of the resource as types.ResourceWithLabels.
func (a *ExternalAuditStorage) CloneResource() types.ResourceWithLabels {
return a.Clone()
}
2 changes: 1 addition & 1 deletion e
Submodule e updated from bb1418 to 5b1208
16 changes: 2 additions & 14 deletions lib/auth/integration/integrationv1/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,19 +248,7 @@ func (s *Service) DeleteIntegration(ctx context.Context, req *integrationpb.Dele
}

// DeleteAllIntegrations removes all Integration resources.
// DEPRECATED: can't delete all integrations over gRPC.
func (s *Service) DeleteAllIntegrations(ctx context.Context, _ *integrationpb.DeleteAllIntegrationsRequest) (*emptypb.Empty, error) {
authCtx, err := s.authorizer.Authorize(ctx)
if err != nil {
return nil, trace.Wrap(err)
}

if err := authCtx.CheckAccessToKind(types.KindIntegration, types.VerbDelete); err != nil {
return nil, trace.Wrap(err)
}

if err := s.backend.DeleteAllIntegrations(ctx); err != nil {
return nil, trace.Wrap(err)
}

return &emptypb.Empty{}, nil
return nil, trace.BadParameter("DeleteAllIntegrations is deprecated")
}
92 changes: 72 additions & 20 deletions lib/auth/integration/integrationv1/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (

integrationpb "github.com/gravitational/teleport/api/gen/proto/go/teleport/integration/v1"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/api/types/externalauditstorage"
"github.com/gravitational/teleport/lib/auth/keystore"
"github.com/gravitational/teleport/lib/auth/testauthority"
"github.com/gravitational/teleport/lib/authz"
Expand Down Expand Up @@ -65,6 +66,7 @@ func TestIntegrationCRUD(t *testing.T) {
Role types.RoleSpecV6
Setup func(t *testing.T, igName string)
Test func(ctx context.Context, resourceSvc *Service, igName string) error
Cleanup func(t *testing.T, igName string)
ErrAssertion func(error) bool
}{
// Read
Expand Down Expand Up @@ -227,7 +229,7 @@ func TestIntegrationCRUD(t *testing.T) {
ErrAssertion: trace.IsAccessDenied,
},
{
Name: "access to delete integration",
Name: "cant delete integration referenced by draft external audit storage",
Role: types.RoleSpecV6{
Allow: types.RoleConditions{Rules: []types.Rule{{
Resources: []string{types.KindIntegration},
Expand All @@ -237,56 +239,96 @@ func TestIntegrationCRUD(t *testing.T) {
Setup: func(t *testing.T, igName string) {
_, err := localClient.CreateIntegration(ctx, sampleIntegrationFn(t, igName))
require.NoError(t, err)
_, err = localClient.GenerateDraftExternalAuditStorage(ctx, igName, "us-west-2")
require.NoError(t, err)
},
Test: func(ctx context.Context, resourceSvc *Service, igName string) error {
_, err := resourceSvc.DeleteIntegration(ctx, &integrationpb.DeleteIntegrationRequest{Name: igName})
return err

},
ErrAssertion: noError,
Cleanup: func(t *testing.T, igName string) {
err := localClient.DeleteDraftExternalAuditStorage(ctx)
require.NoError(t, err)
},
ErrAssertion: trace.IsBadParameter,
},

// Delete all
{
Name: "remove all integrations fails when no access",
Role: types.RoleSpecV6{},
Name: "cant delete integration referenced by cluster external audit storage",
Role: types.RoleSpecV6{
Allow: types.RoleConditions{Rules: []types.Rule{{
Resources: []string{types.KindIntegration},
Verbs: []string{types.VerbDelete},
}}},
},
Setup: func(t *testing.T, igName string) {
_, err := localClient.CreateIntegration(ctx, sampleIntegrationFn(t, igName))
require.NoError(t, err)
_, err = localClient.GenerateDraftExternalAuditStorage(ctx, igName, "us-west-2")
require.NoError(t, err)
err = localClient.PromoteToClusterExternalAuditStorage(ctx)
require.NoError(t, err)
},
Test: func(ctx context.Context, resourceSvc *Service, igName string) error {
_, err := resourceSvc.DeleteAllIntegrations(ctx, &integrationpb.DeleteAllIntegrationsRequest{})
_, err := resourceSvc.DeleteIntegration(ctx, &integrationpb.DeleteIntegrationRequest{Name: igName})
return err
},
ErrAssertion: trace.IsAccessDenied,
Cleanup: func(t *testing.T, igName string) {
err := localClient.DisableClusterExternalAuditStorage(ctx)
require.NoError(t, err)
},
ErrAssertion: trace.IsBadParameter,
},
{
Name: "remove all integrations",
Name: "access to delete integration",
Role: types.RoleSpecV6{
Allow: types.RoleConditions{Rules: []types.Rule{{
Resources: []string{types.KindIntegration},
Verbs: []string{types.VerbDelete},
}}},
},
Setup: func(t *testing.T, _ string) {
for i := 0; i < 10; i++ {
_, err := localClient.CreateIntegration(ctx, sampleIntegrationFn(t, uuid.NewString()))
require.NoError(t, err)
}
Setup: func(t *testing.T, igName string) {
_, err := localClient.CreateIntegration(ctx, sampleIntegrationFn(t, igName))
require.NoError(t, err)
},
Test: func(ctx context.Context, resourceSvc *Service, igName string) error {
_, err := resourceSvc.DeleteAllIntegrations(ctx, &integrationpb.DeleteAllIntegrationsRequest{})
_, err := resourceSvc.DeleteIntegration(ctx, &integrationpb.DeleteIntegrationRequest{Name: igName})
return err
},
ErrAssertion: noError,
},

// Delete all
{
Name: "delete all integrations fails",
Role: types.RoleSpecV6{
Allow: types.RoleConditions{Rules: []types.Rule{{
Resources: []string{types.KindIntegration},
Verbs: []string{types.VerbDelete},
}}},
},
Test: func(ctx context.Context, resourceSvc *Service, igName string) error {
_, err := resourceSvc.DeleteAllIntegrations(ctx, &integrationpb.DeleteAllIntegrationsRequest{})
return err
},
// Deleting all integrations via gRPC is not supported.
ErrAssertion: trace.IsBadParameter,
},
}

for _, tc := range tt {
tc := tc
t.Run(tc.Name, func(t *testing.T) {
localCtx := authorizerForDummyUser(t, ctx, tc.Role, localClient)

igName := uuid.NewString()
if tc.Setup != nil {
tc.Setup(t, igName)
}

if tc.Cleanup != nil {
t.Cleanup(func() { tc.Cleanup(t, igName) })
}

err := tc.Test(localCtx, resourceSvc, igName)
require.True(t, tc.ErrAssertion(err), err)
})
Expand Down Expand Up @@ -322,6 +364,10 @@ type localClient interface {
CreateUser(ctx context.Context, user types.User) (types.User, error)
CreateRole(ctx context.Context, role types.Role) (types.Role, error)
CreateIntegration(ctx context.Context, ig types.Integration) (types.Integration, error)
GenerateDraftExternalAuditStorage(ctx context.Context, integrationName, region string) (*externalauditstorage.ExternalAuditStorage, error)
DeleteDraftExternalAuditStorage(ctx context.Context) error
PromoteToClusterExternalAuditStorage(ctx context.Context) error
DisableClusterExternalAuditStorage(ctx context.Context) error
}

type testClient struct {
Expand All @@ -341,6 +387,7 @@ func initSvc(t *testing.T, ca types.CertAuthority, clusterName string, proxyPubl
trustSvc := local.NewCAService(backend)
roleSvc := local.NewAccessService(backend)
userSvc := local.NewTestIdentityService(backend)
easSvc := local.NewExternalAuditStorageService(backend)

_, err = clusterConfigSvc.UpsertAuthPreference(ctx, types.DefaultAuthPreference())
require.NoError(t, err)
Expand Down Expand Up @@ -378,6 +425,9 @@ func initSvc(t *testing.T, ca types.CertAuthority, clusterName string, proxyPubl
localResourceService, err := local.NewIntegrationsService(backend)
require.NoError(t, err)

cacheResourceService, err := local.NewIntegrationsService(backend, local.WithIntegrationsServiceCacheMode(true))
require.NoError(t, err)

keystoreManager, err := keystore.NewManager(ctx, keystore.Config{
Software: keystore.SoftwareConfig{
RSAKeyPairSource: testauthority.New().GenerateKeyPair,
Expand All @@ -393,7 +443,7 @@ func initSvc(t *testing.T, ca types.CertAuthority, clusterName string, proxyPubl
PublicAddrs: []string{proxyPublicAddr},
}},
},
IntegrationsService: *localResourceService,
IntegrationsService: *cacheResourceService,
}

resourceSvc, err := NewService(&ServiceConfig{
Expand All @@ -407,11 +457,13 @@ func initSvc(t *testing.T, ca types.CertAuthority, clusterName string, proxyPubl
return ctx, struct {
*local.AccessService
*local.IdentityService
*local.ExternalAuditStorageService
*local.IntegrationsService
}{
AccessService: roleSvc,
IdentityService: userSvc,
IntegrationsService: localResourceService,
AccessService: roleSvc,
IdentityService: userSvc,
ExternalAuditStorageService: easSvc,
IntegrationsService: localResourceService,
}, resourceSvc
}

Expand Down
4 changes: 2 additions & 2 deletions lib/backend/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func FlagKey(parts ...string) []byte {
return internalKey(flagsPrefix, parts...)
}

func lockKey(parts ...string) []byte {
func LockKey(parts ...string) []byte {
return internalKey(locksPrefix, parts...)
}

Expand Down Expand Up @@ -88,7 +88,7 @@ func AcquireLock(ctx context.Context, cfg LockConfiguration) (Lock, error) {
if err != nil {
return Lock{}, trace.Wrap(err)
}
key := lockKey(cfg.LockName)
key := LockKey(cfg.LockName)
id, err := randomID()
if err != nil {
return Lock{}, trace.Wrap(err)
Expand Down
2 changes: 1 addition & 1 deletion lib/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -878,7 +878,7 @@ func New(config Config) (*Cache, error) {
return nil, trace.Wrap(err)
}

integrationsCache, err := local.NewIntegrationsService(config.Backend)
integrationsCache, err := local.NewIntegrationsService(config.Backend, local.WithIntegrationsServiceCacheMode(true))
if err != nil {
cancel()
return nil, trace.Wrap(err)
Expand Down
2 changes: 1 addition & 1 deletion lib/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ func newPackWithoutCache(dir string, opts ...packOption) (*testPack, error) {
}
p.okta = oktaSvc

igSvc, err := local.NewIntegrationsService(p.backend)
igSvc, err := local.NewIntegrationsService(p.backend, local.WithIntegrationsServiceCacheMode(true))
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down
4 changes: 2 additions & 2 deletions lib/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -6187,13 +6187,13 @@ func (process *TeleportProcess) newExternalAuditStorageConfigurator() (*external
// watcher initialized.
watcher.WaitInit(process.GracefulExitContext())

ecaSvc := local.NewExternalAuditStorageService(process.backend)
easSvc := local.NewExternalAuditStorageService(process.backend)
integrationSvc, err := local.NewIntegrationsService(process.backend)
if err != nil {
return nil, trace.Wrap(err)
}
statusService := local.NewStatusService(process.backend)
return externalauditstorage.NewConfigurator(process.ExitContext(), ecaSvc, integrationSvc, statusService)
return externalauditstorage.NewConfigurator(process.ExitContext(), easSvc, integrationSvc, statusService)
}

// createLockedPIDFile creates a PID file in the path specified by pidFile
Expand Down
10 changes: 5 additions & 5 deletions lib/service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -504,17 +504,17 @@ func TestAthenaAuditLogSetup(t *testing.T) {
_, err = integrationSvc.CreateIntegration(ctx, oidcIntegration)
require.NoError(t, err)

ecaSvc := local.NewExternalAuditStorageService(backend)
_, err = ecaSvc.GenerateDraftExternalAuditStorage(ctx, "aws-integration-1", "us-west-2")
easSvc := local.NewExternalAuditStorageService(backend)
_, err = easSvc.GenerateDraftExternalAuditStorage(ctx, "aws-integration-1", "us-west-2")
require.NoError(t, err)

statusService := local.NewStatusService(process.backend)

externalAuditStorageDisabled, err := externalauditstorage.NewConfigurator(ctx, ecaSvc, integrationSvc, statusService)
externalAuditStorageDisabled, err := externalauditstorage.NewConfigurator(ctx, easSvc, integrationSvc, statusService)
require.NoError(t, err)
err = ecaSvc.PromoteToClusterExternalAuditStorage(ctx)
err = easSvc.PromoteToClusterExternalAuditStorage(ctx)
require.NoError(t, err)
externalAuditStorageEnabled, err := externalauditstorage.NewConfigurator(ctx, ecaSvc, integrationSvc, statusService)
externalAuditStorageEnabled, err := externalauditstorage.NewConfigurator(ctx, easSvc, integrationSvc, statusService)
require.NoError(t, err)

tests := []struct {
Expand Down