From 80c71abc3008a681c5cd7228d3409a0efe2d2b1d Mon Sep 17 00:00:00 2001 From: Tim Ross Date: Thu, 14 Nov 2024 12:19:11 -0500 Subject: [PATCH] Sanitize aws service custom endpoints There is a slight behavior change in aws-sdk-go-v2 regarding custom endpoints. The new sdk requires the endpoints to be a valid URI, and the legacy sdk would allow a hostname only enpoint and automatically apply a scheme. To prevent breaking existing configurations when upgrading to v17, we now have to apply the same logic to endpoints manually that the legacy sdk used to apply for us. Closes https://github.com/gravitational/teleport/issues/48760. --- lib/events/dynamoevents/dynamoevents.go | 5 ++ lib/events/dynamoevents/dynamoevents_test.go | 12 ++-- lib/events/s3sessions/s3handler.go | 2 + .../s3sessions/s3handler_config_test.go | 12 ++-- lib/utils/aws/endpoint/endpoint.go | 45 ++++++++++++++ lib/utils/aws/endpoint/endpoint_test.go | 58 +++++++++++++++++++ 6 files changed, 126 insertions(+), 8 deletions(-) create mode 100644 lib/utils/aws/endpoint/endpoint.go create mode 100644 lib/utils/aws/endpoint/endpoint_test.go diff --git a/lib/events/dynamoevents/dynamoevents.go b/lib/events/dynamoevents/dynamoevents.go index 992c0b6118016..d17c8895709ac 100644 --- a/lib/events/dynamoevents/dynamoevents.go +++ b/lib/events/dynamoevents/dynamoevents.go @@ -151,6 +151,9 @@ type Config struct { // CredentialsProvider if supplied is used to override the credentials source. CredentialsProvider aws.CredentialsProvider + + // Insecure is an optional switch to opt out of https connections + Insecure bool } // SetFromURL sets values on the Config from the supplied URI @@ -205,6 +208,8 @@ func (cfg *Config) CheckAndSetDefaults() error { cfg.UIDGenerator = utils.NewRealUID() } + cfg.Endpoint = endpoint.CreateURI(cfg.Endpoint, cfg.Insecure) + return nil } diff --git a/lib/events/dynamoevents/dynamoevents_test.go b/lib/events/dynamoevents/dynamoevents_test.go index c8aabdf39c497..8d8c2b5050da9 100644 --- a/lib/events/dynamoevents/dynamoevents_test.go +++ b/lib/events/dynamoevents/dynamoevents_test.go @@ -640,10 +640,14 @@ func TestEndpoints(t *testing.T) { t.Cleanup(server.Close) b, err := New(context.Background(), Config{ - Region: "us-west-1", - Tablename: "teleport-test", - UIDGenerator: utils.NewFakeUID(), - Endpoint: server.URL, + Region: "us-west-1", + Tablename: "teleport-test", + UIDGenerator: utils.NewFakeUID(), + // The prefix is intentionally removed to validate that a scheme + // is applied automatically. This validates backwards compatible behavior + // with existing configurations and the behavior change from aws-sdk-go to aws-sdk-go-v2. + Endpoint: strings.TrimPrefix(server.URL, "http://"), + Insecure: true, UseFIPSEndpoint: fips, CredentialsProvider: aws.CredentialsProviderFunc(func(ctx context.Context) (aws.Credentials, error) { return aws.Credentials{}, nil diff --git a/lib/events/s3sessions/s3handler.go b/lib/events/s3sessions/s3handler.go index e29056806a484..97605db6dfbad 100644 --- a/lib/events/s3sessions/s3handler.go +++ b/lib/events/s3sessions/s3handler.go @@ -157,6 +157,8 @@ func (s *Config) CheckAndSetDefaults() error { return trace.BadParameter("missing parameter Bucket") } + s.Endpoint = endpoint.CreateURI(s.Endpoint, s.Insecure) + return nil } diff --git a/lib/events/s3sessions/s3handler_config_test.go b/lib/events/s3sessions/s3handler_config_test.go index 3689f35229f43..d6af6a6df109b 100644 --- a/lib/events/s3sessions/s3handler_config_test.go +++ b/lib/events/s3sessions/s3handler_config_test.go @@ -24,6 +24,7 @@ import ( "net/http/httptest" "net/url" "os" + "strings" "sync" "testing" @@ -190,10 +191,13 @@ func TestEndpoints(t *testing.T) { t.Cleanup(server.Close) handler, err := NewHandler(context.Background(), Config{ - Region: "us-west-1", - Path: "/test/", - Bucket: "teleport-unit-tests", - Endpoint: server.URL, + Region: "us-west-1", + Path: "/test/", + Bucket: "teleport-unit-tests", + // The prefix is intentionally removed to validate that a scheme + // is applied automatically. This validates backwards compatible behavior + // with existing configurations and the behavior change from aws-sdk-go to aws-sdk-go-v2. + Endpoint: strings.TrimPrefix(server.URL, "http://"), UseFIPSEndpoint: fips, Insecure: true, CredentialsProvider: aws.CredentialsProviderFunc(func(ctx context.Context) (aws.Credentials, error) { diff --git a/lib/utils/aws/endpoint/endpoint.go b/lib/utils/aws/endpoint/endpoint.go new file mode 100644 index 0000000000000..22fddea4e5bd4 --- /dev/null +++ b/lib/utils/aws/endpoint/endpoint.go @@ -0,0 +1,45 @@ +// Teleport +// Copyright (C) 2024 Gravitational, Inc. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package endpoint + +import ( + "fmt" + "regexp" +) + +var schemeRegex = regexp.MustCompile("^([^:]+)://") + +// CreateURI ensures that the provided endpoint is a valid +// URI and contains a scheme. This is primarily to preserve +// backward compatible behavior when changing between +// aws-sdk-go and aws-sdk-go/v2. The legacy sdk automatically +// applied the scheme to custom endpoints, while the new sdk +// does not, and will return an error if the URI is invalid. +// To allow existing configurations to continue to work with +// a custom service endpoint, this performs applies the same +// behavior that the legacy sdk did. +func CreateURI(endpoint string, insecure bool) string { + if !schemeRegex.MatchString(endpoint) { + scheme := "https" + if insecure { + scheme = "http" + } + return fmt.Sprintf("%s://%s", scheme, endpoint) + } + + return endpoint +} diff --git a/lib/utils/aws/endpoint/endpoint_test.go b/lib/utils/aws/endpoint/endpoint_test.go new file mode 100644 index 0000000000000..1b5b89236a682 --- /dev/null +++ b/lib/utils/aws/endpoint/endpoint_test.go @@ -0,0 +1,58 @@ +// Teleport +// Copyright (C) 2024 Gravitational, Inc. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package endpoint_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/gravitational/teleport/lib/utils/aws/endpoint" +) + +func TestCreateURI(t *testing.T) { + tests := []struct { + name string + endpoint string + insecure bool + expected string + }{ + { + name: "valid endpoint", + endpoint: "https://test.example.com", + expected: "https://test.example.com", + }, + { + name: "invalid insecure endpoint", + endpoint: "test.example.com", + insecure: true, + expected: "http://test.example.com", + }, + { + name: "invalid secure endpoint", + endpoint: "test.example.com", + expected: "https://test.example.com", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + out := endpoint.CreateURI(test.endpoint, test.insecure) + require.Equal(t, test.expected, out) + }) + } +}