From 5b519489c1af136ec802c98af1e3539606619062 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 11 Aug 2026 19:06:12 -0700 Subject: [PATCH] remote_storage: build all S3-compatible clients through one constructor (#10720) * remote_storage: build S3-compatible clients through one constructor The eight non-s3 S3-SDK providers each duplicated the AWS session setup and only the s3 maker could take a custom *http.Client. Route every S3-compatible type (s3, wasabi, b2, aliyun, tencent, baidu, filebase, storj, contabo) through MakeWithHTTPClient with a single options table, and add S3CompatibleEndpoint so callers can resolve the endpoint a given type dials. No behavior change. * volume: apply the remote-endpoint check to all S3-compatible providers FetchAndWriteNeedle validated the endpoint and used the pinned dialer only for type "s3". Every S3-SDK backend (wasabi, b2, aliyun, tencent, baidu, filebase, storj, contabo) dials a caller-supplied endpoint through the same client, so gate on S3CompatibleEndpoint to apply the same check uniformly. -volume.allowUntrustedRemoteEndpoints still opts out. * volume: don't route the guarded remote-endpoint client through a proxy The guarded client exists to dial the validated endpoint directly and re-check the resolved IP at connect time. With http.ProxyFromEnvironment set, the dialer only validates the proxy's address while the proxy re-resolves the endpoint host, which reopens the rebinding window. Drop the proxy on this path; operators that need one can opt out with -volume.allowUntrustedRemoteEndpoints. --- weed/remote_storage/s3/aliyun.go | 32 +--- weed/remote_storage/s3/backblaze.go | 27 +-- weed/remote_storage/s3/baidu.go | 34 +--- weed/remote_storage/s3/contabo.go | 32 +--- weed/remote_storage/s3/endpoint_test.go | 44 +++++ weed/remote_storage/s3/filebase.go | 34 +--- weed/remote_storage/s3/s3_storage_client.go | 174 ++++++++++++++++-- .../s3/s3_storage_client_test.go | 2 + weed/remote_storage/s3/storj.go | 32 +--- weed/remote_storage/s3/tencent.go | 32 +--- weed/remote_storage/s3/wasabi.go | 44 +---- weed/server/volume_grpc_remote.go | 33 ++-- weed/server/volume_grpc_remote_test.go | 39 ++++ 13 files changed, 274 insertions(+), 285 deletions(-) create mode 100644 weed/remote_storage/s3/endpoint_test.go diff --git a/weed/remote_storage/s3/aliyun.go b/weed/remote_storage/s3/aliyun.go index 6954c2669..f32a5c030 100644 --- a/weed/remote_storage/s3/aliyun.go +++ b/weed/remote_storage/s3/aliyun.go @@ -1,16 +1,8 @@ package s3 import ( - "fmt" - "os" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/credentials" - "github.com/aws/aws-sdk-go/aws/session" - "github.com/aws/aws-sdk-go/service/s3" "github.com/seaweedfs/seaweedfs/weed/pb/remote_pb" "github.com/seaweedfs/seaweedfs/weed/remote_storage" - "github.com/seaweedfs/seaweedfs/weed/util" ) func init() { @@ -24,27 +16,5 @@ func (s AliyunRemoteStorageMaker) HasBucket() bool { } func (s AliyunRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.RemoteStorageClient, error) { - client := &s3RemoteStorageClient{ - conf: conf, - } - accessKey := util.Nvl(conf.AliyunAccessKey, os.Getenv("ALICLOUD_ACCESS_KEY_ID")) - secretKey := util.Nvl(conf.AliyunSecretKey, os.Getenv("ALICLOUD_ACCESS_KEY_SECRET")) - - config := &aws.Config{ - Endpoint: aws.String(conf.AliyunEndpoint), - Region: aws.String(conf.AliyunRegion), - S3ForcePathStyle: aws.Bool(false), - S3DisableContentMD5Validation: aws.Bool(true), - } - if accessKey != "" && secretKey != "" { - config.Credentials = credentials.NewStaticCredentials(accessKey, secretKey, "") - } - - sess, err := session.NewSession(config) - if err != nil { - return nil, fmt.Errorf("create aliyun session: %w", err) - } - sess.Handlers.Build.PushFront(skipSha256PayloadSigning) - client.conn = s3.New(sess) - return client, nil + return MakeWithHTTPClient(conf, nil) } diff --git a/weed/remote_storage/s3/backblaze.go b/weed/remote_storage/s3/backblaze.go index d8cfbeb15..64a50e636 100644 --- a/weed/remote_storage/s3/backblaze.go +++ b/weed/remote_storage/s3/backblaze.go @@ -1,12 +1,6 @@ package s3 import ( - "fmt" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/credentials" - "github.com/aws/aws-sdk-go/aws/session" - "github.com/aws/aws-sdk-go/service/s3" "github.com/seaweedfs/seaweedfs/weed/pb/remote_pb" "github.com/seaweedfs/seaweedfs/weed/remote_storage" ) @@ -22,24 +16,5 @@ func (s BackBlazeRemoteStorageMaker) HasBucket() bool { } func (s BackBlazeRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.RemoteStorageClient, error) { - client := &s3RemoteStorageClient{ - conf: conf, - } - config := &aws.Config{ - Endpoint: aws.String(conf.BackblazeEndpoint), - Region: aws.String(conf.BackblazeRegion), - S3ForcePathStyle: aws.Bool(true), - S3DisableContentMD5Validation: aws.Bool(true), - } - if conf.BackblazeKeyId != "" && conf.BackblazeApplicationKey != "" { - config.Credentials = credentials.NewStaticCredentials(conf.BackblazeKeyId, conf.BackblazeApplicationKey, "") - } - - sess, err := session.NewSession(config) - if err != nil { - return nil, fmt.Errorf("create backblaze session: %w", err) - } - sess.Handlers.Build.PushFront(skipSha256PayloadSigning) - client.conn = s3.New(sess) - return client, nil + return MakeWithHTTPClient(conf, nil) } diff --git a/weed/remote_storage/s3/baidu.go b/weed/remote_storage/s3/baidu.go index 285e095c3..7844c7c0f 100644 --- a/weed/remote_storage/s3/baidu.go +++ b/weed/remote_storage/s3/baidu.go @@ -1,17 +1,8 @@ package s3 import ( - "fmt" - "os" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/credentials" - "github.com/aws/aws-sdk-go/aws/session" - v4 "github.com/aws/aws-sdk-go/aws/signer/v4" - "github.com/aws/aws-sdk-go/service/s3" "github.com/seaweedfs/seaweedfs/weed/pb/remote_pb" "github.com/seaweedfs/seaweedfs/weed/remote_storage" - "github.com/seaweedfs/seaweedfs/weed/util" ) func init() { @@ -25,28 +16,5 @@ func (s BaiduRemoteStorageMaker) HasBucket() bool { } func (s BaiduRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.RemoteStorageClient, error) { - client := &s3RemoteStorageClient{ - conf: conf, - } - accessKey := util.Nvl(conf.BaiduAccessKey, os.Getenv("BDCLOUD_ACCESS_KEY")) - secretKey := util.Nvl(conf.BaiduSecretKey, os.Getenv("BDCLOUD_SECRET_KEY")) - - config := &aws.Config{ - Endpoint: aws.String(conf.BaiduEndpoint), - Region: aws.String(conf.BaiduRegion), - S3ForcePathStyle: aws.Bool(false), - S3DisableContentMD5Validation: aws.Bool(true), - } - if accessKey != "" && secretKey != "" { - config.Credentials = credentials.NewStaticCredentials(accessKey, secretKey, "") - } - - sess, err := session.NewSession(config) - if err != nil { - return nil, fmt.Errorf("create baidu session: %w", err) - } - sess.Handlers.Sign.PushBackNamed(v4.SignRequestHandler) - sess.Handlers.Build.PushFront(skipSha256PayloadSigning) - client.conn = s3.New(sess) - return client, nil + return MakeWithHTTPClient(conf, nil) } diff --git a/weed/remote_storage/s3/contabo.go b/weed/remote_storage/s3/contabo.go index d20a8ec0a..2a0883fb4 100644 --- a/weed/remote_storage/s3/contabo.go +++ b/weed/remote_storage/s3/contabo.go @@ -1,16 +1,8 @@ package s3 import ( - "fmt" - "os" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/credentials" - "github.com/aws/aws-sdk-go/aws/session" - "github.com/aws/aws-sdk-go/service/s3" "github.com/seaweedfs/seaweedfs/weed/pb/remote_pb" "github.com/seaweedfs/seaweedfs/weed/remote_storage" - "github.com/seaweedfs/seaweedfs/weed/util" ) func init() { @@ -24,27 +16,5 @@ func (s ContaboRemoteStorageMaker) HasBucket() bool { } func (s ContaboRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.RemoteStorageClient, error) { - client := &s3RemoteStorageClient{ - conf: conf, - } - accessKey := util.Nvl(conf.ContaboAccessKey, os.Getenv("ACCESS_KEY")) - secretKey := util.Nvl(conf.ContaboSecretKey, os.Getenv("SECRET_KEY")) - - config := &aws.Config{ - Endpoint: aws.String(conf.ContaboEndpoint), - Region: aws.String(conf.ContaboRegion), - S3ForcePathStyle: aws.Bool(true), - S3DisableContentMD5Validation: aws.Bool(true), - } - if accessKey != "" && secretKey != "" { - config.Credentials = credentials.NewStaticCredentials(accessKey, secretKey, "") - } - - sess, err := session.NewSession(config) - if err != nil { - return nil, fmt.Errorf("create contabo session: %w", err) - } - sess.Handlers.Build.PushFront(skipSha256PayloadSigning) - client.conn = s3.New(sess) - return client, nil + return MakeWithHTTPClient(conf, nil) } diff --git a/weed/remote_storage/s3/endpoint_test.go b/weed/remote_storage/s3/endpoint_test.go new file mode 100644 index 000000000..85a9c0682 --- /dev/null +++ b/weed/remote_storage/s3/endpoint_test.go @@ -0,0 +1,44 @@ +package s3 + +import ( + "testing" + + "github.com/seaweedfs/seaweedfs/weed/pb/remote_pb" +) + +// TestS3CompatibleEndpointCoversAllProviders locks the type-to-endpoint mapping +// the volume server's SSRF guard relies on. Every S3-SDK-backed provider dials +// a caller-supplied endpoint, so each must surface it here; a new sibling that +// forgets to would silently bypass the guard. +func TestS3CompatibleEndpointCoversAllProviders(t *testing.T) { + cases := []struct { + conf *remote_pb.RemoteConf + want string + }{ + {&remote_pb.RemoteConf{Type: "s3", S3Endpoint: "http://s3.internal"}, "http://s3.internal"}, + {&remote_pb.RemoteConf{Type: "wasabi", WasabiEndpoint: "http://wasabi.internal"}, "http://wasabi.internal"}, + {&remote_pb.RemoteConf{Type: "b2", BackblazeEndpoint: "http://b2.internal"}, "http://b2.internal"}, + {&remote_pb.RemoteConf{Type: "aliyun", AliyunEndpoint: "http://aliyun.internal"}, "http://aliyun.internal"}, + {&remote_pb.RemoteConf{Type: "tencent", TencentEndpoint: "http://tencent.internal"}, "http://tencent.internal"}, + {&remote_pb.RemoteConf{Type: "baidu", BaiduEndpoint: "http://baidu.internal"}, "http://baidu.internal"}, + {&remote_pb.RemoteConf{Type: "filebase", FilebaseEndpoint: "http://filebase.internal"}, "http://filebase.internal"}, + {&remote_pb.RemoteConf{Type: "storj", StorjEndpoint: "http://storj.internal"}, "http://storj.internal"}, + {&remote_pb.RemoteConf{Type: "contabo", ContaboEndpoint: "http://contabo.internal"}, "http://contabo.internal"}, + } + for _, tc := range cases { + endpoint, ok := S3CompatibleEndpoint(tc.conf) + if !ok { + t.Errorf("type %q: expected an S3-compatible endpoint, got ok=false", tc.conf.Type) + continue + } + if endpoint != tc.want { + t.Errorf("type %q: endpoint = %q, want %q", tc.conf.Type, endpoint, tc.want) + } + } + + // Every registered maker must be an S3-compatible type; the guard treats + // anything else (gcs, azure, ...) as not dialing a caller-supplied URL. + if _, ok := S3CompatibleEndpoint(&remote_pb.RemoteConf{Type: "gcs"}); ok { + t.Error("gcs must not be reported as an S3-compatible endpoint") + } +} diff --git a/weed/remote_storage/s3/filebase.go b/weed/remote_storage/s3/filebase.go index 89b96e385..23fc50834 100644 --- a/weed/remote_storage/s3/filebase.go +++ b/weed/remote_storage/s3/filebase.go @@ -1,17 +1,8 @@ package s3 import ( - "fmt" - "os" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/credentials" - "github.com/aws/aws-sdk-go/aws/session" - v4 "github.com/aws/aws-sdk-go/aws/signer/v4" - "github.com/aws/aws-sdk-go/service/s3" "github.com/seaweedfs/seaweedfs/weed/pb/remote_pb" "github.com/seaweedfs/seaweedfs/weed/remote_storage" - "github.com/seaweedfs/seaweedfs/weed/util" ) func init() { @@ -25,28 +16,5 @@ func (s FilebaseRemoteStorageMaker) HasBucket() bool { } func (s FilebaseRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.RemoteStorageClient, error) { - client := &s3RemoteStorageClient{ - conf: conf, - } - accessKey := util.Nvl(conf.FilebaseAccessKey, os.Getenv("AWS_ACCESS_KEY_ID")) - secretKey := util.Nvl(conf.FilebaseSecretKey, os.Getenv("AWS_SECRET_ACCESS_KEY")) - - config := &aws.Config{ - Endpoint: aws.String(conf.FilebaseEndpoint), - Region: aws.String("us-east-1"), - S3ForcePathStyle: aws.Bool(true), - S3DisableContentMD5Validation: aws.Bool(true), - } - if accessKey != "" && secretKey != "" { - config.Credentials = credentials.NewStaticCredentials(accessKey, secretKey, "") - } - - sess, err := session.NewSession(config) - if err != nil { - return nil, fmt.Errorf("create filebase session: %w", err) - } - sess.Handlers.Sign.PushBackNamed(v4.SignRequestHandler) - sess.Handlers.Build.PushFront(skipSha256PayloadSigning) - client.conn = s3.New(sess) - return client, nil + return MakeWithHTTPClient(conf, nil) } diff --git a/weed/remote_storage/s3/s3_storage_client.go b/weed/remote_storage/s3/s3_storage_client.go index 1ca606633..5aa4f2d69 100644 --- a/weed/remote_storage/s3/s3_storage_client.go +++ b/weed/remote_storage/s3/s3_storage_client.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "net/url" + "os" "reflect" "strings" @@ -41,45 +42,188 @@ func (s s3RemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.R return MakeWithHTTPClient(conf, nil) } -// MakeWithHTTPClient builds an s3 remote storage client using the supplied -// *http.Client (or the AWS SDK default when nil). Callers that need to pin -// the dial path against DNS rebinding can pass a client whose transport has -// a guarded DialContext. +// s3CompatibleOptions carries the per-provider AWS SDK knobs the shared client +// builder needs. Every S3-SDK-backed provider (s3 plus the wasabi/b2/... family) +// converges on the same s3RemoteStorageClient and dials a caller-supplied +// endpoint, differing only in which RemoteConf fields it reads. +type s3CompatibleOptions struct { + name string + endpoint string + region string + accessKey string + secretKey string + forcePathStyle bool + signV4 bool + // anonymousWhenNoCreds disables SigV4 signing for public buckets when no + // credentials are supplied. Only the generic "s3" provider does this. + anonymousWhenNoCreds bool + // setUserAgent adds the SeaweedFS User-Agent header (generic "s3" only). + setUserAgent bool +} + +// s3CompatibleClientOptions returns the client knobs for an S3-SDK-backed +// RemoteConf and whether conf is such a type. Keeping the type-to-fields +// mapping in one place lets the client builder and the SSRF endpoint guard +// agree on exactly which endpoint each provider dials. +func s3CompatibleClientOptions(conf *remote_pb.RemoteConf) (s3CompatibleOptions, bool) { + switch conf.Type { + case "s3": + return s3CompatibleOptions{ + name: "s3", + endpoint: conf.S3Endpoint, + region: conf.S3Region, + accessKey: conf.S3AccessKey, + secretKey: conf.S3SecretKey, + forcePathStyle: conf.S3ForcePathStyle, + signV4: conf.S3V4Signature, + anonymousWhenNoCreds: true, + setUserAgent: true, + }, true + case "aliyun": + return s3CompatibleOptions{ + name: "aliyun", + endpoint: conf.AliyunEndpoint, + region: conf.AliyunRegion, + accessKey: util.Nvl(conf.AliyunAccessKey, os.Getenv("ALICLOUD_ACCESS_KEY_ID")), + secretKey: util.Nvl(conf.AliyunSecretKey, os.Getenv("ALICLOUD_ACCESS_KEY_SECRET")), + }, true + case "b2": + return s3CompatibleOptions{ + name: "backblaze", + endpoint: conf.BackblazeEndpoint, + region: conf.BackblazeRegion, + accessKey: conf.BackblazeKeyId, + secretKey: conf.BackblazeApplicationKey, + forcePathStyle: true, + }, true + case "baidu": + return s3CompatibleOptions{ + name: "baidu", + endpoint: conf.BaiduEndpoint, + region: conf.BaiduRegion, + accessKey: util.Nvl(conf.BaiduAccessKey, os.Getenv("BDCLOUD_ACCESS_KEY")), + secretKey: util.Nvl(conf.BaiduSecretKey, os.Getenv("BDCLOUD_SECRET_KEY")), + signV4: true, + }, true + case "contabo": + return s3CompatibleOptions{ + name: "contabo", + endpoint: conf.ContaboEndpoint, + region: conf.ContaboRegion, + accessKey: util.Nvl(conf.ContaboAccessKey, os.Getenv("ACCESS_KEY")), + secretKey: util.Nvl(conf.ContaboSecretKey, os.Getenv("SECRET_KEY")), + forcePathStyle: true, + }, true + case "filebase": + return s3CompatibleOptions{ + name: "filebase", + endpoint: conf.FilebaseEndpoint, + region: "us-east-1", + accessKey: util.Nvl(conf.FilebaseAccessKey, os.Getenv("AWS_ACCESS_KEY_ID")), + secretKey: util.Nvl(conf.FilebaseSecretKey, os.Getenv("AWS_SECRET_ACCESS_KEY")), + forcePathStyle: true, + signV4: true, + }, true + case "storj": + return s3CompatibleOptions{ + name: "storj", + endpoint: conf.StorjEndpoint, + region: "us-west-2", + accessKey: util.Nvl(conf.StorjAccessKey, os.Getenv("AWS_ACCESS_KEY_ID")), + secretKey: util.Nvl(conf.StorjSecretKey, os.Getenv("AWS_SECRET_ACCESS_KEY")), + forcePathStyle: true, + }, true + case "tencent": + return s3CompatibleOptions{ + name: "tencent", + endpoint: conf.TencentEndpoint, + region: "us-west-2", + accessKey: util.Nvl(conf.TencentSecretId, os.Getenv("COS_SECRETID")), + secretKey: util.Nvl(conf.TencentSecretKey, os.Getenv("COS_SECRETKEY")), + forcePathStyle: true, + }, true + case "wasabi": + return s3CompatibleOptions{ + name: "wasabi", + endpoint: conf.WasabiEndpoint, + region: conf.WasabiRegion, + accessKey: conf.WasabiAccessKey, + secretKey: conf.WasabiSecretKey, + forcePathStyle: true, + }, true + } + return s3CompatibleOptions{}, false +} + +// S3CompatibleEndpoint returns the endpoint an S3-SDK-backed RemoteConf would +// dial directly, and whether conf is such a type. The volume server validates +// this endpoint against the SSRF deny-list for every S3-compatible provider, +// not just the generic "s3" type. +func S3CompatibleEndpoint(conf *remote_pb.RemoteConf) (string, bool) { + opt, ok := s3CompatibleClientOptions(conf) + if !ok { + return "", false + } + return opt.endpoint, true +} + +// MakeWithHTTPClient builds the client for any S3-SDK-backed remote storage +// type using the supplied *http.Client (or the AWS SDK default when nil). +// Callers that need to pin the dial path against DNS rebinding can pass a +// client whose transport has a guarded DialContext. func MakeWithHTTPClient(conf *remote_pb.RemoteConf, httpClient *http.Client) (remote_storage.RemoteStorageClient, error) { + opt, ok := s3CompatibleClientOptions(conf) + if !ok { + return nil, fmt.Errorf("%q is not an S3-compatible remote storage type", conf.Type) + } client := &s3RemoteStorageClient{ conf: conf, } config := &aws.Config{ - Region: aws.String(conf.S3Region), - Endpoint: aws.String(conf.S3Endpoint), - S3ForcePathStyle: aws.Bool(conf.S3ForcePathStyle), + Region: aws.String(opt.region), + Endpoint: aws.String(opt.endpoint), + S3ForcePathStyle: aws.Bool(opt.forcePathStyle), S3DisableContentMD5Validation: aws.Bool(true), } if httpClient != nil { config.HTTPClient = httpClient } - if conf.S3AccessKey != "" && conf.S3SecretKey != "" { - config.Credentials = credentials.NewStaticCredentials(conf.S3AccessKey, conf.S3SecretKey, "") - } else if conf.S3AccessKey == "" && conf.S3SecretKey == "" { + if opt.accessKey != "" && opt.secretKey != "" { + config.Credentials = credentials.NewStaticCredentials(opt.accessKey, opt.secretKey, "") + } else if opt.anonymousWhenNoCreds && opt.accessKey == "" && opt.secretKey == "" { // Explicitly disable signing for public buckets. config.Credentials = credentials.AnonymousCredentials } sess, err := session.NewSession(config) if err != nil { - return nil, fmt.Errorf("create aws session: %w", err) + return nil, fmt.Errorf("create %s session: %w", opt.name, err) } - if conf.S3V4Signature { + if opt.signV4 { sess.Handlers.Sign.PushBackNamed(v4.SignRequestHandler) } - sess.Handlers.Build.PushBack(func(r *request.Request) { - r.HTTPRequest.Header.Set("User-Agent", "SeaweedFS/"+version.VERSION_NUMBER) - }) + if opt.setUserAgent { + sess.Handlers.Build.PushBack(func(r *request.Request) { + r.HTTPRequest.Header.Set("User-Agent", "SeaweedFS/"+version.VERSION_NUMBER) + }) + } sess.Handlers.Build.PushFront(skipSha256PayloadSigning) client.conn = s3.New(sess) return client, nil } +var skipSha256PayloadSigning = func(r *request.Request) { + // see https://github.com/ceph/ceph/pull/15965/files + if r.ClientInfo.ServiceID != "S3" { + return + } + if r.Operation.Name == "PutObject" || r.Operation.Name == "UploadPart" { + if len(r.HTTPRequest.Header.Get("X-Amz-Content-Sha256")) == 0 { + r.HTTPRequest.Header.Set("X-Amz-Content-Sha256", "UNSIGNED-PAYLOAD") + } + } +} + type s3RemoteStorageClient struct { conf *remote_pb.RemoteConf conn s3iface.S3API diff --git a/weed/remote_storage/s3/s3_storage_client_test.go b/weed/remote_storage/s3/s3_storage_client_test.go index 40bc56fb0..33f381d33 100644 --- a/weed/remote_storage/s3/s3_storage_client_test.go +++ b/weed/remote_storage/s3/s3_storage_client_test.go @@ -228,6 +228,7 @@ func newCapturingS3Client(t *testing.T) (*s3RemoteStorageClient, *captureRoundTr t.Helper() rt := &captureRoundTripper{} conf := &remote_pb.RemoteConf{ + Type: "s3", Name: "test", S3Region: "us-east-1", S3Endpoint: "https://example.invalid", @@ -281,6 +282,7 @@ func newRecordingS3Client(t *testing.T, supportTagging bool) (*s3RemoteStorageCl t.Helper() rt := &recordingRoundTripper{} conf := &remote_pb.RemoteConf{ + Type: "s3", Name: "test", S3Region: "us-east-1", S3Endpoint: "https://example.invalid", diff --git a/weed/remote_storage/s3/storj.go b/weed/remote_storage/s3/storj.go index 9ec403d61..4674b944a 100644 --- a/weed/remote_storage/s3/storj.go +++ b/weed/remote_storage/s3/storj.go @@ -1,16 +1,8 @@ package s3 import ( - "fmt" - "os" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/credentials" - "github.com/aws/aws-sdk-go/aws/session" - "github.com/aws/aws-sdk-go/service/s3" "github.com/seaweedfs/seaweedfs/weed/pb/remote_pb" "github.com/seaweedfs/seaweedfs/weed/remote_storage" - "github.com/seaweedfs/seaweedfs/weed/util" ) func init() { @@ -24,27 +16,5 @@ func (s StorjRemoteStorageMaker) HasBucket() bool { } func (s StorjRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.RemoteStorageClient, error) { - client := &s3RemoteStorageClient{ - conf: conf, - } - accessKey := util.Nvl(conf.StorjAccessKey, os.Getenv("AWS_ACCESS_KEY_ID")) - secretKey := util.Nvl(conf.StorjSecretKey, os.Getenv("AWS_SECRET_ACCESS_KEY")) - - config := &aws.Config{ - Endpoint: aws.String(conf.StorjEndpoint), - Region: aws.String("us-west-2"), - S3ForcePathStyle: aws.Bool(true), - S3DisableContentMD5Validation: aws.Bool(true), - } - if accessKey != "" && secretKey != "" { - config.Credentials = credentials.NewStaticCredentials(accessKey, secretKey, "") - } - - sess, err := session.NewSession(config) - if err != nil { - return nil, fmt.Errorf("create storj session: %w", err) - } - sess.Handlers.Build.PushFront(skipSha256PayloadSigning) - client.conn = s3.New(sess) - return client, nil + return MakeWithHTTPClient(conf, nil) } diff --git a/weed/remote_storage/s3/tencent.go b/weed/remote_storage/s3/tencent.go index 184a5b372..1fdd58b77 100644 --- a/weed/remote_storage/s3/tencent.go +++ b/weed/remote_storage/s3/tencent.go @@ -1,16 +1,8 @@ package s3 import ( - "fmt" - "os" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/credentials" - "github.com/aws/aws-sdk-go/aws/session" - "github.com/aws/aws-sdk-go/service/s3" "github.com/seaweedfs/seaweedfs/weed/pb/remote_pb" "github.com/seaweedfs/seaweedfs/weed/remote_storage" - "github.com/seaweedfs/seaweedfs/weed/util" ) func init() { @@ -24,27 +16,5 @@ func (s TencentRemoteStorageMaker) HasBucket() bool { } func (s TencentRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.RemoteStorageClient, error) { - client := &s3RemoteStorageClient{ - conf: conf, - } - accessKey := util.Nvl(conf.TencentSecretId, os.Getenv("COS_SECRETID")) - secretKey := util.Nvl(conf.TencentSecretKey, os.Getenv("COS_SECRETKEY")) - - config := &aws.Config{ - Endpoint: aws.String(conf.TencentEndpoint), - Region: aws.String("us-west-2"), - S3ForcePathStyle: aws.Bool(true), - S3DisableContentMD5Validation: aws.Bool(true), - } - if accessKey != "" && secretKey != "" { - config.Credentials = credentials.NewStaticCredentials(accessKey, secretKey, "") - } - - sess, err := session.NewSession(config) - if err != nil { - return nil, fmt.Errorf("create tencent session: %w", err) - } - sess.Handlers.Build.PushFront(skipSha256PayloadSigning) - client.conn = s3.New(sess) - return client, nil + return MakeWithHTTPClient(conf, nil) } diff --git a/weed/remote_storage/s3/wasabi.go b/weed/remote_storage/s3/wasabi.go index 5b98485ca..7649b2377 100644 --- a/weed/remote_storage/s3/wasabi.go +++ b/weed/remote_storage/s3/wasabi.go @@ -1,16 +1,8 @@ package s3 import ( - "fmt" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/credentials" - "github.com/aws/aws-sdk-go/aws/request" - "github.com/aws/aws-sdk-go/aws/session" - "github.com/aws/aws-sdk-go/service/s3" "github.com/seaweedfs/seaweedfs/weed/pb/remote_pb" "github.com/seaweedfs/seaweedfs/weed/remote_storage" - "github.com/seaweedfs/seaweedfs/weed/util" ) func init() { @@ -24,39 +16,5 @@ func (s WasabiRemoteStorageMaker) HasBucket() bool { } func (s WasabiRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.RemoteStorageClient, error) { - client := &s3RemoteStorageClient{ - conf: conf, - } - accessKey := util.Nvl(conf.WasabiAccessKey) - secretKey := util.Nvl(conf.WasabiSecretKey) - - config := &aws.Config{ - Endpoint: aws.String(conf.WasabiEndpoint), - Region: aws.String(conf.WasabiRegion), - S3ForcePathStyle: aws.Bool(true), - S3DisableContentMD5Validation: aws.Bool(true), - } - if accessKey != "" && secretKey != "" { - config.Credentials = credentials.NewStaticCredentials(accessKey, secretKey, "") - } - - sess, err := session.NewSession(config) - if err != nil { - return nil, fmt.Errorf("create wasabi session: %w", err) - } - sess.Handlers.Build.PushFront(skipSha256PayloadSigning) - client.conn = s3.New(sess) - return client, nil -} - -var skipSha256PayloadSigning = func(r *request.Request) { - // see https://github.com/ceph/ceph/pull/15965/files - if r.ClientInfo.ServiceID != "S3" { - return - } - if r.Operation.Name == "PutObject" || r.Operation.Name == "UploadPart" { - if len(r.HTTPRequest.Header.Get("X-Amz-Content-Sha256")) == 0 { - r.HTTPRequest.Header.Set("X-Amz-Content-Sha256", "UNSIGNED-PAYLOAD") - } - } + return MakeWithHTTPClient(conf, nil) } diff --git a/weed/server/volume_grpc_remote.go b/weed/server/volume_grpc_remote.go index 36fe5c683..472df49d1 100644 --- a/weed/server/volume_grpc_remote.go +++ b/weed/server/volume_grpc_remote.go @@ -202,7 +202,12 @@ func guardedDialer(endpoint string) func(ctx context.Context, network, addr stri func newGuardedHTTPClient(endpoint string) *http.Client { return &http.Client{ Transport: &http.Transport{ - Proxy: http.ProxyFromEnvironment, + // No proxy: guardedDialer must see the real target address. Through + // a proxy it would only validate the proxy's IP while the proxy + // re-resolves the endpoint host, reopening the rebinding window the + // dialer exists to close. Operators that need a proxy can opt out + // with -volume.allowUntrustedRemoteEndpoints. + Proxy: nil, DialContext: guardedDialer(endpoint), ForceAttemptHTTP2: true, MaxIdleConns: 16, @@ -229,22 +234,28 @@ func (vs *VolumeServer) FetchAndWriteNeedle(ctx context.Context, req *volume_ser remoteConf := req.RemoteConf + // Every S3-SDK-backed backend (s3, wasabi, b2, storj, contabo, tencent, + // aliyun, baidu, filebase) dials this caller-supplied endpoint directly, + // so the guard must cover all of them, not just type "s3". Other backends + // (gcs, azure, ...) authenticate against their own SDKs and don't accept + // an attacker-controlled host. + endpoint, isS3Compatible := "", false + if remoteConf != nil { + endpoint, isS3Compatible = s3remote.S3CompatibleEndpoint(remoteConf) + } + var client remote_storage.RemoteStorageClient var getClientErr error - if !vs.AllowUntrustedRemoteEndpoints && remoteConf != nil && remoteConf.Type == "s3" { - // Endpoint validation is S3-specific: only RemoteConf.S3Endpoint - // is a URL the volume server dials directly. Other backends - // (gcs, azure, ...) authenticate against their own SDKs and - // don't accept an attacker-controlled host. - if validateErr := validateRemoteEndpoint(ctx, remoteConf.S3Endpoint); validateErr != nil { + if !vs.AllowUntrustedRemoteEndpoints && isS3Compatible { + if validateErr := validateRemoteEndpoint(ctx, endpoint); validateErr != nil { return nil, fmt.Errorf("reject remote endpoint: %w", validateErr) } - // Build a one-shot S3 client whose dial path re-validates the - // resolved IP every time. This pins the validated endpoint against - // DNS rebinding (a hostname that resolves to a public IP for + // Build a one-shot client whose dial path re-validates the resolved + // IP every time. This pins the validated endpoint against DNS + // rebinding (a hostname that resolves to a public IP for // validateRemoteEndpoint and then flips to 127.0.0.1 / 169.254.x.x // when the AWS SDK dials). - client, getClientErr = s3remote.MakeWithHTTPClient(remoteConf, newGuardedHTTPClient(remoteConf.S3Endpoint)) + client, getClientErr = s3remote.MakeWithHTTPClient(remoteConf, newGuardedHTTPClient(endpoint)) } else { client, getClientErr = remote_storage.GetRemoteStorage(remoteConf) } diff --git a/weed/server/volume_grpc_remote_test.go b/weed/server/volume_grpc_remote_test.go index 229c32f9c..141d2eb9c 100644 --- a/weed/server/volume_grpc_remote_test.go +++ b/weed/server/volume_grpc_remote_test.go @@ -7,6 +7,9 @@ import ( "strings" "sync/atomic" "testing" + + "github.com/seaweedfs/seaweedfs/weed/pb/remote_pb" + s3remote "github.com/seaweedfs/seaweedfs/weed/remote_storage/s3" ) // stubLookup returns a resolver func that maps the supplied hostnames to @@ -303,6 +306,42 @@ func TestGuardedDialerRebind(t *testing.T) { } } +// TestRemoteEndpointGuardCoversS3CompatibleSiblings confirms the SSRF guard +// reaches every S3-SDK-backed provider, not just type "s3". It replays the two +// steps FetchAndWriteNeedle performs before building the client: resolve the +// endpoint the type would dial, then validate it against the deny-list. A +// sibling type pointed at an internal address must be rejected. +func TestRemoteEndpointGuardCoversS3CompatibleSiblings(t *testing.T) { + cases := []struct { + conf *remote_pb.RemoteConf + wantSub string + }{ + {&remote_pb.RemoteConf{Type: "wasabi", WasabiEndpoint: "http://169.254.169.254/"}, "metadata"}, + {&remote_pb.RemoteConf{Type: "b2", BackblazeEndpoint: "http://127.0.0.1/"}, "loopback"}, + {&remote_pb.RemoteConf{Type: "aliyun", AliyunEndpoint: "http://192.168.0.1/"}, "private"}, + {&remote_pb.RemoteConf{Type: "tencent", TencentEndpoint: "http://100.64.0.1/"}, "CGNAT"}, + {&remote_pb.RemoteConf{Type: "baidu", BaiduEndpoint: "http://169.254.169.254/"}, "metadata"}, + {&remote_pb.RemoteConf{Type: "filebase", FilebaseEndpoint: "http://172.16.0.1/"}, "private"}, + {&remote_pb.RemoteConf{Type: "storj", StorjEndpoint: "http://10.0.0.5/"}, "private"}, + {&remote_pb.RemoteConf{Type: "contabo", ContaboEndpoint: "http://[::1]/"}, "loopback"}, + } + for _, tc := range cases { + endpoint, ok := s3remote.S3CompatibleEndpoint(tc.conf) + if !ok { + t.Errorf("type %q: not recognized as S3-compatible, guard would be skipped", tc.conf.Type) + continue + } + err := validateRemoteEndpoint(context.Background(), endpoint) + if err == nil { + t.Errorf("type %q: expected endpoint %q to be rejected", tc.conf.Type, endpoint) + continue + } + if !strings.Contains(err.Error(), tc.wantSub) { + t.Errorf("type %q: error %q missing %q", tc.conf.Type, err, tc.wantSub) + } + } +} + // TestGuardedDialerLiteralBlocked confirms that a literal blocked IP target // is refused without any DNS lookup. func TestGuardedDialerLiteralBlocked(t *testing.T) {