diff --git a/docs/pages/admin-guides/teleport-policy/integrations/aws-sync.mdx b/docs/pages/admin-guides/teleport-policy/integrations/aws-sync.mdx index 5341c70af34b3..421a732a44384 100644 --- a/docs/pages/admin-guides/teleport-policy/integrations/aws-sync.mdx +++ b/docs/pages/admin-guides/teleport-policy/integrations/aws-sync.mdx @@ -156,6 +156,8 @@ The IAM policy includes the following directives: "s3:ListBucket", "s3:GetBucketLocation", "s3:GetBucketTagging", + "s3:GetBucketPolicyStatus", + "s3:GetBucketAcl", "iam:ListUsers", "iam:GetUser", diff --git a/lib/cloud/aws/policy_statements.go b/lib/cloud/aws/policy_statements.go index 644981e3ccdab..d7fb9520d728b 100644 --- a/lib/cloud/aws/policy_statements.go +++ b/lib/cloud/aws/policy_statements.go @@ -406,6 +406,9 @@ func StatementAccessGraphAWSSync() *Statement { "s3:ListBucket", "s3:GetBucketLocation", "s3:GetBucketTagging", + "s3:GetBucketPolicyStatus", + "s3:GetBucketAcl", + // IAM IAM "iam:ListUsers", "iam:GetUser", diff --git a/lib/cloud/mocks/aws_s3.go b/lib/cloud/mocks/aws_s3.go index f68b76d294963..8c49ea4471f07 100644 --- a/lib/cloud/mocks/aws_s3.go +++ b/lib/cloud/mocks/aws_s3.go @@ -35,6 +35,7 @@ type S3Mock struct { BucketPolicyStatus map[string]*s3.PolicyStatus BucketACL map[string][]*s3.Grant BucketTags map[string][]*s3.Tag + BucketLocations map[string]string } func (m *S3Mock) ListBucketsWithContext(_ aws.Context, _ *s3.ListBucketsInput, _ ...request.Option) (*s3.ListBucketsOutput, error) { @@ -94,3 +95,16 @@ func (m *S3Mock) GetBucketTaggingWithContext(_ aws.Context, input *s3.GetBucketT TagSet: tags, }, nil } + +func (m *S3Mock) GetBucketLocationWithContext(_ aws.Context, input *s3.GetBucketLocationInput, _ ...request.Option) (*s3.GetBucketLocationOutput, error) { + if aws.StringValue(input.Bucket) == "" { + return nil, trace.BadParameter("incorrect bucket name") + } + location, ok := m.BucketLocations[aws.StringValue(input.Bucket)] + if !ok { + return nil, trace.NotFound("bucket %v not found", aws.StringValue(input.Bucket)) + } + return &s3.GetBucketLocationOutput{ + LocationConstraint: aws.String(location), + }, nil +} diff --git a/lib/integrations/awsoidc/testdata/TestAccessGraphAWSIAMConfigOuput.golden b/lib/integrations/awsoidc/testdata/TestAccessGraphAWSIAMConfigOuput.golden index 488cea7437994..82e937184fe78 100644 --- a/lib/integrations/awsoidc/testdata/TestAccessGraphAWSIAMConfigOuput.golden +++ b/lib/integrations/awsoidc/testdata/TestAccessGraphAWSIAMConfigOuput.golden @@ -32,6 +32,8 @@ PutRolePolicy: { "s3:ListBucket", "s3:GetBucketLocation", "s3:GetBucketTagging", + "s3:GetBucketPolicyStatus", + "s3:GetBucketAcl", "iam:ListUsers", "iam:GetUser", "iam:ListRoles", diff --git a/lib/srv/discovery/fetchers/aws-sync/s3.go b/lib/srv/discovery/fetchers/aws-sync/s3.go index a1e83b33d2ce7..75f2645aa0308 100644 --- a/lib/srv/discovery/fetchers/aws-sync/s3.go +++ b/lib/srv/discovery/fetchers/aws-sync/s3.go @@ -71,28 +71,13 @@ func (a *awsFetcher) fetchS3Buckets(ctx context.Context) ([]*accessgraphv1alpha. } } - region := awsutil.GetKnownRegions()[0] - if len(a.Regions) > 0 { - region = a.Regions[0] - } - - s3Client, err := a.CloudClients.GetAWSS3Client( - ctx, - region, - a.getAWSOptions()..., - ) - if err != nil { - return nil, trace.Wrap(err) - } - rsp, err := s3Client.ListBucketsWithContext( - ctx, - &s3.ListBucketsInput{}, - ) + buckets, getBucketRegion, err := a.listS3Buckets(ctx) if err != nil { return existing.S3Buckets, trace.Wrap(err) } - for _, bucket := range rsp.Buckets { + // Iterate over the buckets and fetch their inline and attached policies. + for _, bucket := range buckets { bucket := bucket eG.Go(func() error { var failedReqs failedRequests @@ -101,54 +86,24 @@ func (a *awsFetcher) fetchS3Buckets(ctx context.Context) ([]*accessgraphv1alpha. return b.Name == aws.ToString(bucket.Name) && b.AccountId == a.AccountID }, ) - policy, err := s3Client.GetBucketPolicyWithContext(ctx, &s3.GetBucketPolicyInput{ - Bucket: bucket.Name, - }) + bucketRegion, err := getBucketRegion(bucket.Name) if err != nil { errs = append(errs, - trace.Wrap(err, "failed to fetch bucket %q inline policy", aws.ToString(bucket.Name)), + trace.Wrap(err), ) failedReqs.policyFailed = true - } - - policyStatus, err := s3Client.GetBucketPolicyStatusWithContext(ctx, &s3.GetBucketPolicyStatusInput{ - Bucket: bucket.Name, - }) - if err != nil { - errs = append(errs, - trace.Wrap(err, "failed to fetch bucket %q policy status", aws.ToString(bucket.Name)), - ) failedReqs.failedPolicyStatus = true - } - - acls, err := s3Client.GetBucketAclWithContext(ctx, &s3.GetBucketAclInput{ - Bucket: bucket.Name, - }) - if err != nil { - errs = append(errs, - trace.Wrap(err, "failed to fetch bucket %q acls policies", aws.ToString(bucket.Name)), - ) failedReqs.failedAcls = true - } - - tagsOutput, err := s3Client.GetBucketTaggingWithContext(ctx, &s3.GetBucketTaggingInput{ - Bucket: bucket.Name, - }) - var awsErr awserr.Error - const noSuchTagSet = "NoSuchTagSet" // error code when there are no tags or the bucket does not support them - if errors.As(err, &awsErr) && awsErr.Code() == noSuchTagSet { - // If there are no tags, set the error to nil. - err = nil - } - if err != nil { - errs = append(errs, - trace.Wrap(err, "failed to fetch bucket %q tags", aws.ToString(bucket.Name)), - ) failedReqs.failedTags = true + newBucket := awsS3Bucket(aws.ToString(bucket.Name), nil, nil, nil, nil, a.AccountID) + collect(mergeS3Protos(existingBucket, newBucket, failedReqs), trace.NewAggregate(errs...)) + return nil } - newBucket := awsS3Bucket(aws.ToString(bucket.Name), policy, policyStatus, acls, tagsOutput, a.AccountID) - collect(mergeS3Protos(existingBucket, newBucket, failedReqs), trace.NewAggregate(errs...)) + details, failedReqs, errsL := a.getS3BucketDetails(ctx, bucket, bucketRegion) + + newBucket := awsS3Bucket(aws.ToString(bucket.Name), details.policy, details.policyStatus, details.acls, details.tags, a.AccountID) + collect(mergeS3Protos(existingBucket, newBucket, failedReqs), trace.NewAggregate(append(errs, errsL...)...)) return nil }) } @@ -212,6 +167,7 @@ type failedRequests struct { failedPolicyStatus bool failedAcls bool failedTags bool + headFailed bool } func mergeS3Protos(existing, new *accessgraphv1alpha.AWSS3BucketV1, failedReqs failedRequests) *accessgraphv1alpha.AWSS3BucketV1 { @@ -237,3 +193,124 @@ func mergeS3Protos(existing, new *accessgraphv1alpha.AWSS3BucketV1, failedReqs f return clone } + +type s3Details struct { + policy *s3.GetBucketPolicyOutput + policyStatus *s3.GetBucketPolicyStatusOutput + acls *s3.GetBucketAclOutput + tags *s3.GetBucketTaggingOutput +} + +func (a *awsFetcher) getS3BucketDetails(ctx context.Context, bucket *s3.Bucket, bucketRegion string) (s3Details, failedRequests, []error) { + var failedReqs failedRequests + var errs []error + var details s3Details + + s3Client, err := a.CloudClients.GetAWSS3Client( + ctx, + bucketRegion, + a.getAWSOptions()..., + ) + if err != nil { + errs = append(errs, + trace.Wrap(err, "failed to create s3 client for bucket %q", aws.ToString(bucket.Name)), + ) + return s3Details{}, + failedRequests{ + headFailed: true, + policyFailed: true, + failedPolicyStatus: true, + failedAcls: true, + failedTags: true, + }, errs + } + + details.policy, err = s3Client.GetBucketPolicyWithContext(ctx, &s3.GetBucketPolicyInput{ + Bucket: bucket.Name, + }) + if err != nil && !isS3BucketPolicyNotFound(err) { + errs = append(errs, + trace.Wrap(err, "failed to fetch bucket %q inline policy", aws.ToString(bucket.Name)), + ) + failedReqs.policyFailed = true + } + + details.policyStatus, err = s3Client.GetBucketPolicyStatusWithContext(ctx, &s3.GetBucketPolicyStatusInput{ + Bucket: bucket.Name, + }) + if err != nil && !isS3BucketPolicyNotFound(err) { + errs = append(errs, + trace.Wrap(err, "failed to fetch bucket %q policy status", aws.ToString(bucket.Name)), + ) + failedReqs.failedPolicyStatus = true + } + + details.acls, err = s3Client.GetBucketAclWithContext(ctx, &s3.GetBucketAclInput{ + Bucket: bucket.Name, + }) + if err != nil { + errs = append(errs, + trace.Wrap(err, "failed to fetch bucket %q acls policies", aws.ToString(bucket.Name)), + ) + failedReqs.failedAcls = true + } + + details.tags, err = s3Client.GetBucketTaggingWithContext(ctx, &s3.GetBucketTaggingInput{ + Bucket: bucket.Name, + }) + if err != nil && !isS3BucketNoTagSet(err) { + errs = append(errs, + trace.Wrap(err, "failed to fetch bucket %q tags", aws.ToString(bucket.Name)), + ) + failedReqs.failedTags = true + } + + return details, failedReqs, errs +} + +func isS3BucketPolicyNotFound(err error) bool { + var awsErr awserr.Error + return errors.As(err, &awsErr) && awsErr.Code() == "NoSuchBucketPolicy" +} + +func isS3BucketNoTagSet(err error) bool { + var awsErr awserr.Error + return errors.As(err, &awsErr) && awsErr.Code() == "NoSuchTagSet" +} + +func (a *awsFetcher) listS3Buckets(ctx context.Context) ([]*s3.Bucket, func(*string) (string, error), error) { + region := awsutil.GetKnownRegions()[0] + if len(a.Regions) > 0 { + region = a.Regions[0] + } + + // use any region to list buckets + s3Client, err := a.CloudClients.GetAWSS3Client( + ctx, + region, + a.getAWSOptions()..., + ) + if err != nil { + return nil, nil, trace.Wrap(err) + } + rsp, err := s3Client.ListBucketsWithContext(ctx, &s3.ListBucketsInput{}) + if err != nil { + return nil, nil, trace.Wrap(err) + } + return rsp.Buckets, + func(bucket *string) (string, error) { + rsp, err := s3Client.GetBucketLocationWithContext( + ctx, + &s3.GetBucketLocationInput{ + Bucket: bucket, + }, + ) + if err != nil { + return "", trace.Wrap(err, "failed to fetch bucket %q region", aws.ToString(bucket)) + } + if rsp.LocationConstraint == nil { + return "us-east-1", nil + } + return aws.ToString(rsp.LocationConstraint), nil + }, nil +} diff --git a/lib/srv/discovery/fetchers/aws-sync/s3_test.go b/lib/srv/discovery/fetchers/aws-sync/s3_test.go index 318943a97e038..b20b22d4d4ffe 100644 --- a/lib/srv/discovery/fetchers/aws-sync/s3_test.go +++ b/lib/srv/discovery/fetchers/aws-sync/s3_test.go @@ -55,6 +55,10 @@ func TestPollAWSS3(t *testing.T) { mockedClients = &cloud.TestCloudClients{ S3: &mocks.S3Mock{ Buckets: s3Buckets(bucketName, otherBucketName), + BucketLocations: map[string]string{ + bucketName: "eu-west-1", + otherBucketName: "eu-west-1", + }, BucketPolicy: map[string]string{ bucketName: "policy", otherBucketName: "otherPolicy",