Skip to content

Commit

Permalink
Restore S3 FIPS and custom endpoint capabilities
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 committed Nov 8, 2024
1 parent 0b13107 commit 3fc7673
Show file tree
Hide file tree
Showing 8 changed files with 206 additions and 87 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
76 changes: 58 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,62 @@ 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,
})
}

var request *http.Request
mux := http.NewServeMux()
mux.Handle("/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
request = r
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 {
require.Error(t, err)
return
}

assert.Error(t, err)
assert.Nil(t, b)
require.ErrorContains(t, err, fmt.Sprintf("StatusCode: %d", http.StatusTeapot))
require.Equal(t, "/", request.URL.Path)
})
}
}
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
64 changes: 64 additions & 0 deletions lib/events/s3sessions/s3handler_config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,17 @@ package s3sessions

import (
"context"
"net/http"
"net/http/httptest"
"net/url"
"os"
"testing"

"github.com/aws/aws-sdk-go-v2/aws"
"github.com/stretchr/testify/require"

"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/modules"
"github.com/gravitational/teleport/lib/utils"
)

Expand Down Expand Up @@ -133,3 +137,63 @@ func TestUploadMetadata(t *testing.T) {
meta := handler.GetUploadMetadata("test-session-id")
require.Equal(t, "s3://teleport-unit-tests/test/test-session-id", meta.URL)
}

func TestEndpoints(t *testing.T) {
tests := []struct {
name string
fips bool
}{
{
name: "fips",
fips: true,
},
{
name: "without fips",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
fips := types.ClusterAuditConfigSpecV2_FIPS_DISABLED
if tt.fips {
fips = types.ClusterAuditConfigSpecV2_FIPS_ENABLED
modules.SetTestModules(t, &modules.TestModules{
FIPS: true,
})
}

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

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

handler, err := NewHandler(context.Background(), Config{
Region: "us-west-1",
Path: "/test/",
Bucket: "teleport-unit-tests",
Endpoint: server.URL,
UseFIPSEndpoint: fips,
Insecure: true,
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 {
require.Error(t, err)
return
}

require.NoError(t, err)
defer handler.Close()

require.Equal(t, "/teleport-unit-tests", request.URL.Path)
})
}
}
Loading

0 comments on commit 3fc7673

Please sign in to comment.