Skip to content

Commit

Permalink
Restore S3 FIPS and custom endpoint capabilities (#48684)
Browse files Browse the repository at this point in the history 
When migrating the S3 events handler to use aws-sdk-go-v2 applying
the FIPS settings and custom endpoint were inadvertently dropped.
This restores the functionality, while also adding tests to ensure
that they are always respected going forward.

A similar test was added to the dynamodb events handler as well to
prevent any regressions with FIPS settings there.
  • Loading branch information
@rosstimothy
rosstimothy authored Nov 12, 2024
1 parent fa2e416 commit 56d85fa
Show file tree
Hide file tree
Showing 8 changed files with 227 additions and 90 deletions.
31 changes: 14 additions & 17 deletions lib/events/dynamoevents/dynamoevents.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import (
"github.com/aws/aws-sdk-go-v2/service/dynamodb"
dynamodbtypes "github.com/aws/aws-sdk-go-v2/service/dynamodb/types"
"github.com/aws/smithy-go"
smithyendpoints "github.com/aws/smithy-go/endpoints"
"github.com/google/uuid"
"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
Expand Down Expand Up @@ -148,6 +147,9 @@ type Config struct {

// EnableAutoScaling is used to enable auto scaling policy.
EnableAutoScaling bool

// CredentialsProvider if supplied is used to override the credentials source.
CredentialsProvider aws.CredentialsProvider
}

// SetFromURL sets values on the Config from the supplied URI
Expand Down Expand Up @@ -282,24 +284,20 @@ func New(ctx context.Context, cfg Config) (*Log, error) {
config.WithAPIOptions(dynamometrics.MetricsMiddleware(dynamometrics.Backend)),
}

awsConfig, err := config.LoadDefaultConfig(ctx, opts...)
if err != nil {
return nil, trace.Wrap(err)
if cfg.CredentialsProvider != nil {
opts = append(opts, config.WithCredentialsProvider(cfg.CredentialsProvider))
}

otelaws.AppendMiddlewares(&awsConfig.APIOptions, otelaws.WithAttributeSetter(otelaws.DynamoDBAttributeSetter))

var dynamoOpts []func(*dynamodb.Options)

// Override the service endpoint using the "endpoint" query parameter from
// "audit_events_uri". This is for non-AWS DynamoDB-compatible backends.
if cfg.Endpoint != "" {
u, err := url.Parse(cfg.Endpoint)
if err != nil {
if _, err := url.Parse(cfg.Endpoint); err != nil {
return nil, trace.BadParameter("configured DynamoDB events endpoint is invalid: %s", err.Error())
}

dynamoOpts = append(dynamoOpts, dynamodb.WithEndpointResolverV2(&staticResolver{endpoint: u}))
opts = append(opts, config.WithBaseEndpoint(cfg.Endpoint))
}

// FIPS settings are applied on the individual service instead of the aws config,
Expand All @@ -311,6 +309,13 @@ func New(ctx context.Context, cfg Config) (*Log, error) {
})
}

awsConfig, err := config.LoadDefaultConfig(ctx, opts...)
if err != nil {
return nil, trace.Wrap(err)
}

otelaws.AppendMiddlewares(&awsConfig.APIOptions, otelaws.WithAttributeSetter(otelaws.DynamoDBAttributeSetter))

b := &Log{
logger: l,
Config: cfg,
Expand All @@ -324,14 +329,6 @@ func New(ctx context.Context, cfg Config) (*Log, error) {
return b, nil
}

type staticResolver struct {
endpoint *url.URL
}

func (s *staticResolver) ResolveEndpoint(ctx context.Context, params dynamodb.EndpointParameters) (smithyendpoints.Endpoint, error) {
return smithyendpoints.Endpoint{URI: *s.endpoint}, nil
}

type tableStatus int

const (
Expand Down
74 changes: 56 additions & 18 deletions lib/events/dynamoevents/dynamoevents_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"testing"
"time"

"github.com/aws/aws-sdk-go-v2/aws"
"github.com/google/uuid"
"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
Expand All @@ -43,6 +44,7 @@ import (
apievents "github.com/gravitational/teleport/api/types/events"
"github.com/gravitational/teleport/lib/events"
"github.com/gravitational/teleport/lib/events/test"
"github.com/gravitational/teleport/lib/modules"
"github.com/gravitational/teleport/lib/session"
"github.com/gravitational/teleport/lib/utils"
)
Expand Down Expand Up @@ -604,24 +606,60 @@ func randStringAlpha(n int) string {
return string(b)
}

func TestCustomEndpoint(t *testing.T) {
ctx := context.Background()
t.Setenv("AWS_ACCESS_KEY", "llama")
t.Setenv("AWS_SECRET_KEY", "alpaca")
func TestEndpoints(t *testing.T) {
tests := []struct {
name string
fips bool
}{
{
name: "fips",
fips: true,
},
{
name: "without fips",
},
}

mux := http.NewServeMux()
mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusTeapot)
})
srv := httptest.NewServer(mux)
defer srv.Close()
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {

b, err := New(ctx, Config{
Tablename: "teleport-test",
UIDGenerator: utils.NewFakeUID(),
Endpoint: srv.URL,
})
assert.Error(t, err)
assert.Nil(t, b)
require.ErrorContains(t, err, fmt.Sprintf("StatusCode: %d", http.StatusTeapot))
fips := types.ClusterAuditConfigSpecV2_FIPS_DISABLED
if tt.fips {
fips = types.ClusterAuditConfigSpecV2_FIPS_ENABLED
modules.SetTestModules(t, &modules.TestModules{
FIPS: true,
})
}

mux := http.NewServeMux()
mux.Handle("/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusTeapot)
}))

server := httptest.NewServer(mux)
t.Cleanup(server.Close)

b, err := New(context.Background(), Config{
Region: "us-west-1",
Tablename: "teleport-test",
UIDGenerator: utils.NewFakeUID(),
Endpoint: server.URL,
UseFIPSEndpoint: fips,
CredentialsProvider: aws.CredentialsProviderFunc(func(ctx context.Context) (aws.Credentials, error) {
return aws.Credentials{}, nil
}),
})
// FIPS mode should fail because it is a violation to enable FIPS
// while also setting a custom endpoint.
if tt.fips {
assert.Error(t, err)
require.ErrorContains(t, err, "FIPS")
return
}

assert.Error(t, err)
assert.Nil(t, b)
require.ErrorContains(t, err, fmt.Sprintf("StatusCode: %d", http.StatusTeapot))
})
}
}
95 changes: 54 additions & 41 deletions lib/events/s3sessions/s3handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import (
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/events"
"github.com/gravitational/teleport/lib/modules"
awsmetrics "github.com/gravitational/teleport/lib/observability/metrics/aws"
"github.com/gravitational/teleport/lib/session"
awsutils "github.com/gravitational/teleport/lib/utils/aws"
Expand Down Expand Up @@ -77,8 +78,6 @@ type Config struct {
Endpoint string
// ACL is the canned ACL to send to S3
ACL string
// AWSConfig is an optional existing AWS client configuration
AWSConfig *aws.Config
// CredentialsProvider if supplied is used in tests or with External Audit Storage.
CredentialsProvider aws.CredentialsProvider
// SSEKMSKey specifies the optional custom CMK used for KMS SSE.
Expand Down Expand Up @@ -157,55 +156,66 @@ func (s *Config) CheckAndSetDefaults() error {
return trace.BadParameter("missing parameter Bucket")
}

if s.AWSConfig == nil {
var err error
opts := []func(*config.LoadOptions) error{
config.WithRegion(s.Region),
}
return nil
}

if s.Insecure {
opts = append(opts, config.WithHTTPClient(&http.Client{
Transport: &http.Transport{
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
},
}))
} else {
hc, err := defaults.HTTPClient()
if err != nil {
return trace.Wrap(err)
}
// NewHandler returns new S3 uploader
func NewHandler(ctx context.Context, cfg Config) (*Handler, error) {
if err := cfg.CheckAndSetDefaults(); err != nil {
return nil, trace.Wrap(err)
}

opts = append(opts, config.WithHTTPClient(hc))
}
opts := []func(*config.LoadOptions) error{
config.WithRegion(cfg.Region),
}

if s.CredentialsProvider != nil {
opts = append(opts, config.WithCredentialsProvider(s.CredentialsProvider))
if cfg.Insecure {
opts = append(opts, config.WithHTTPClient(&http.Client{
Transport: &http.Transport{
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
},
}))
} else {
hc, err := defaults.HTTPClient()
if err != nil {
return nil, trace.Wrap(err)
}

opts = append(opts, config.WithAPIOptions(awsmetrics.MetricsMiddleware()))
opts = append(opts, config.WithHTTPClient(hc))
}

awsConfig, err := config.LoadDefaultConfig(context.Background(), opts...)
if err != nil {
return trace.Wrap(err)
if cfg.CredentialsProvider != nil {
opts = append(opts, config.WithCredentialsProvider(cfg.CredentialsProvider))
}

opts = append(opts, config.WithAPIOptions(awsmetrics.MetricsMiddleware()))

var s3Opts []func(*s3.Options)
if cfg.Endpoint != "" {
if _, err := url.Parse(cfg.Endpoint); err != nil {
return nil, trace.BadParameter("configured S3 endpoint is invalid: %s", err.Error())
}

s.AWSConfig = &awsConfig
opts = append(opts, config.WithBaseEndpoint(cfg.Endpoint))

s3Opts = append(s3Opts, func(options *s3.Options) {
options.UsePathStyle = true
})
}
return nil
}

// NewHandler returns new S3 uploader
func NewHandler(ctx context.Context, cfg Config) (*Handler, error) {
if err := cfg.CheckAndSetDefaults(); err != nil {
if modules.GetModules().IsBoringBinary() && cfg.UseFIPSEndpoint == types.ClusterAuditConfigSpecV2_FIPS_ENABLED {
s3Opts = append(s3Opts, func(options *s3.Options) {
options.EndpointOptions.UseFIPSEndpoint = aws.FIPSEndpointStateEnabled
})
}

awsConfig, err := config.LoadDefaultConfig(context.Background(), opts...)
if err != nil {
return nil, trace.Wrap(err)
}

// Create S3 client with custom options
client := s3.NewFromConfig(*cfg.AWSConfig, func(o *s3.Options) {
if cfg.Endpoint != "" {
o.UsePathStyle = true
}
})
client := s3.NewFromConfig(awsConfig, s3Opts...)

uploader := manager.NewUploader(client)
downloader := manager.NewDownloader(client)
Expand Down Expand Up @@ -382,14 +392,17 @@ func (h *Handler) ensureBucket(ctx context.Context) error {
Bucket: aws.String(h.Bucket),
})
err = awsutils.ConvertS3Error(err)
// assumes that bucket is administered by other entity
if err == nil {
switch {
case err == nil:
// assumes that bucket is administered by other entity
return nil
}
if !trace.IsNotFound(err) {
case trace.IsBadParameter(err):
return trace.Wrap(err)
case !trace.IsNotFound(err):
h.logger.ErrorContext(ctx, "Failed to ensure that S3 bucket exists. S3 session uploads may fail. If you've set up the bucket already and gave Teleport write-only access, feel free to ignore this error.", "bucket", h.Bucket, "error", err)
return nil
}

input := &s3.CreateBucketInput{
Bucket: aws.String(h.Bucket),
ACL: awstypes.BucketCannedACLPrivate,
Expand Down
Loading

0 comments on commit 56d85fa

Please sign in to comment.