From f5f1dcbd8c5d83e4cde455a1a07071e1c1b6ab18 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Wed, 26 Aug 2026 10:09:05 -0700 Subject: [PATCH] s3: keep verifying the request host when externalUrl is set (#10970) * s3: keep verifying the request host when externalUrl is set externalUrl was the only host candidate once set, so a client that dialed the gateway directly instead of through the proxy always got SignatureDoesNotMatch. Make it lead the candidate walk instead: every candidate still needs a valid signature, and the request-derived hosts are already trusted when the flag is unset, so a mixed proxy plus in-cluster topology can now advertise a public endpoint and verify both planes. * s3: cover virtual-hosted addressing behind externalUrl The old pin also rejected an external client that signed bucket.api.example.com, since only the bare externalUrl host was ever tried. The candidate walk covers it; pin the case down. --- test/s3/proxy_signature/README.md | 10 +++++----- weed/command/filer.go | 2 +- weed/command/mini.go | 2 +- weed/command/s3.go | 2 +- weed/command/server.go | 2 +- weed/s3api/auth_credentials.go | 4 ++-- weed/s3api/auth_security_test.go | 14 ++++++++++++++ weed/s3api/auth_signature_v4.go | 9 +++++---- weed/s3api/auth_signature_v4_test.go | 16 +++++++++++++++- weed/s3api/s3api_server.go | 2 +- 10 files changed, 46 insertions(+), 17 deletions(-) diff --git a/test/s3/proxy_signature/README.md b/test/s3/proxy_signature/README.md index 7a8f5dd81..558dcc6d3 100644 --- a/test/s3/proxy_signature/README.md +++ b/test/s3/proxy_signature/README.md @@ -26,16 +26,16 @@ nginx (:9000) v SeaweedFS S3 (:8333, -s3.externalUrl=http://localhost:9000) | externalHost = "localhost:9000" (parsed at startup) - | extractHostHeader() returns "localhost:9000" + | extractHostHeaderCandidates() tries "localhost:9000" first | Matches what AWS CLI signed with v Signature verification succeeds ``` -**Note:** When `-s3.externalUrl` is configured, direct access to the backend -port (8333) will fail signature verification because the client signs with a -different Host header than what `externalUrl` specifies. This is expected — -all S3 traffic should go through the proxy. +**Note:** `-s3.externalUrl` is tried first, not exclusively. A client that +dials the backend port (8333) directly still verifies against the host it +actually signed, so a mixed topology of proxied and in-cluster clients works +with the flag set. ## Prerequisites diff --git a/weed/command/filer.go b/weed/command/filer.go index 90777925e..a50176f0d 100644 --- a/weed/command/filer.go +++ b/weed/command/filer.go @@ -158,7 +158,7 @@ func init() { filerS3Options.iamReadOnly = cmdFiler.Flag.Bool("s3.iam.readOnly", true, "disable IAM write operations on this server") filerS3Options.portIceberg = cmdFiler.Flag.Int("s3.port.iceberg", 8181, "Iceberg REST Catalog server listen port (0 to disable)") filerS3Options.portLance = cmdFiler.Flag.Int("s3.port.lance", 9101, "Lance Namespace server listen port (0 to disable)") - filerS3Options.externalUrl = cmdFiler.Flag.String("s3.externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Used for S3 signature verification behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.") + filerS3Options.externalUrl = cmdFiler.Flag.String("s3.externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Advertised to Iceberg and Lance clients, and tried first when verifying S3 signatures behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.") filerS3Options.defaultFileMode = cmdFiler.Flag.String("s3.defaultFileMode", "", "default file mode for S3 uploaded objects, e.g. 0660, 0644, 0666") filerS3Options.cacheSizeMB = cmdFiler.Flag.Int64("s3.cacheCapacityMB", 0, "in-memory chunk cache capacity in MB for S3 GETs shared across requests (0 disables)") diff --git a/weed/command/mini.go b/weed/command/mini.go index 8cbc6233b..65f72d761 100644 --- a/weed/command/mini.go +++ b/weed/command/mini.go @@ -526,7 +526,7 @@ func initMiniS3Flags() { miniS3Options.auditLogConfig = cmdMini.Flag.String("s3.auditLogConfig", "", "path to the audit log config file") miniS3Options.allowDeleteBucketNotEmpty = miniS3AllowDeleteBucketNotEmpty miniS3Options.autoCreateBucket = miniS3AutoCreateBucket - miniS3Options.externalUrl = cmdMini.Flag.String("s3.externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Used for S3 signature verification behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.") + miniS3Options.externalUrl = cmdMini.Flag.String("s3.externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Advertised to Iceberg and Lance clients, and tried first when verifying S3 signatures behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.") miniS3Options.defaultFileMode = cmdMini.Flag.String("s3.defaultFileMode", "", "default file mode for S3 uploaded objects, e.g. 0660, 0644, 0666") miniS3Options.cacheSizeMB = cmdMini.Flag.Int64("s3.cacheCapacityMB", 0, "in-memory chunk cache capacity in MB for S3 GETs shared across requests (0 disables)") // In mini mode, S3 uses the shared debug server started at line 681, not its own separate debug server diff --git a/weed/command/s3.go b/weed/command/s3.go index fc8150c5f..e7051401d 100644 --- a/weed/command/s3.go +++ b/weed/command/s3.go @@ -124,7 +124,7 @@ func init() { s3StandaloneOptions.debug = cmdS3.Flag.Bool("debug", false, "serves runtime profiling data via pprof on the port specified by -debug.port") s3StandaloneOptions.debugPort = cmdS3.Flag.Int("debug.port", 6060, "http port for debugging") s3StandaloneOptions.cipher = cmdS3.Flag.Bool("encryptVolumeData", false, "encrypt data on volume servers") - s3StandaloneOptions.externalUrl = cmdS3.Flag.String("externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Used for S3 signature verification behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.") + s3StandaloneOptions.externalUrl = cmdS3.Flag.String("externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Advertised to Iceberg and Lance clients, and tried first when verifying S3 signatures behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.") s3StandaloneOptions.defaultFileMode = cmdS3.Flag.String("defaultFileMode", "", "default file mode for S3 uploaded objects, e.g. 0660, 0644, 0666") s3StandaloneOptions.cacheSizeMB = cmdS3.Flag.Int64("cacheCapacityMB", 0, "in-memory chunk cache capacity in MB for S3 GETs shared across requests (0 disables)") } diff --git a/weed/command/server.go b/weed/command/server.go index 625f87d3d..732121cd4 100644 --- a/weed/command/server.go +++ b/weed/command/server.go @@ -187,7 +187,7 @@ func init() { s3Options.enableIam = cmdServer.Flag.Bool("s3.iam", true, "enable embedded IAM API on the same S3 port") s3Options.iamReadOnly = cmdServer.Flag.Bool("s3.iam.readOnly", true, "disable IAM write operations on this server") s3Options.cipher = cmdServer.Flag.Bool("s3.encryptVolumeData", false, "encrypt data on volume servers for S3 uploads") - s3Options.externalUrl = cmdServer.Flag.String("s3.externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Used for S3 signature verification behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.") + s3Options.externalUrl = cmdServer.Flag.String("s3.externalUrl", "", "the external URL clients use to connect (e.g. https://api.example.com:9000). Advertised to Iceberg and Lance clients, and tried first when verifying S3 signatures behind a reverse proxy. Falls back to S3_EXTERNAL_URL env var.") s3Options.defaultFileMode = cmdServer.Flag.String("s3.defaultFileMode", "", "default file mode for S3 uploaded objects, e.g. 0660, 0644, 0666") s3Options.cacheSizeMB = cmdServer.Flag.Int64("s3.cacheCapacityMB", 0, "in-memory chunk cache capacity in MB for S3 GETs shared across requests (0 disables)") diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index bfea12eba..2eb7d5ec5 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -60,7 +60,7 @@ type IdentityAccessManagement struct { hashCounters map[string]*int32 identityAnonymous *Identity domain string - externalHost string // pre-computed host for S3 signature verification (from ExternalUrl) + externalHost string // pre-computed host tried first during S3 signature verification (from ExternalUrl) isAuthEnabled bool credentialManager *credential.CredentialManager filerClient *wdclient.FilerClient @@ -315,7 +315,7 @@ func NewIdentityAccessManagementWithStore(option *S3ApiServerOption, filerClient if err != nil { glog.Fatalf("failed to parse s3.externalUrl: %v", err) } - glog.V(0).Infof("S3 signature verification will use external host: %q (from %q)", externalHost, option.ExternalUrl) + glog.V(0).Infof("S3 signature verification will try external host %q (from %q) first", externalHost, option.ExternalUrl) } iam := &IdentityAccessManagement{ diff --git a/weed/s3api/auth_security_test.go b/weed/s3api/auth_security_test.go index b6e31089d..24b3c183c 100644 --- a/weed/s3api/auth_security_test.go +++ b/weed/s3api/auth_security_test.go @@ -312,6 +312,20 @@ func TestExternalUrlSignatureVerification(t *testing.T) { externalUrl: "http://api.example.com:80", expectSuccess: true, }, + { + name: "externalUrl set, in-cluster client signs the service host", + clientUrl: "http://seaweedfs-s3:8333/test-bucket/object", + backendHost: "seaweedfs-s3:8333", + externalUrl: "https://api.example.com", + expectSuccess: true, + }, + { + name: "externalUrl set, external client signs a virtual-hosted bucket name", + clientUrl: "https://test-bucket.api.example.com/object", + backendHost: "test-bucket.api.example.com", + externalUrl: "https://api.example.com", + expectSuccess: true, + }, { name: "without externalUrl, internal host causes mismatch", clientUrl: "https://api.example.com:9000/test-bucket/object", diff --git a/weed/s3api/auth_signature_v4.go b/weed/s3api/auth_signature_v4.go index 4d0bf533f..890365725 100644 --- a/weed/s3api/auth_signature_v4.go +++ b/weed/s3api/auth_signature_v4.go @@ -888,14 +888,16 @@ func extractHostHeader(r *http.Request, externalHost string) string { } // extractHostHeaderCandidates returns the host values the client may have signed, most -// likely first. When externalHost is set (from s3.externalUrl), it is the only candidate. -// Otherwise, the host is reconstructed from X-Forwarded-* headers or the request Host. +// likely first. externalHost (from s3.externalUrl) leads when set, but the hosts derived +// from X-Forwarded-* headers or the request Host still follow it, so clients that reach +// the gateway directly rather than through the proxy keep verifying. // When X-Forwarded-Host carries no port, the true client port is ambiguous: a proxy that // kept the Host header makes the r.Host port right, one that rewrote it makes // X-Forwarded-Port right, and a client on the scheme's default port signed no port at all. func extractHostHeaderCandidates(r *http.Request, externalHost string) []string { + var candidates []string if externalHost != "" { - return []string{externalHost} + candidates = append(candidates, externalHost) } forwardedHost := r.Header.Get("X-Forwarded-Host") @@ -968,7 +970,6 @@ func extractHostHeaderCandidates(r *http.Request, externalHost string) []string } } - var candidates []string for _, port := range ports { candidate := joinSignedHost(host, port, scheme) if !slices.Contains(candidates, candidate) { diff --git a/weed/s3api/auth_signature_v4_test.go b/weed/s3api/auth_signature_v4_test.go index 4be2f97aa..d1601b16e 100644 --- a/weed/s3api/auth_signature_v4_test.go +++ b/weed/s3api/auth_signature_v4_test.go @@ -535,9 +535,23 @@ func TestExtractHostHeaderCandidates(t *testing.T) { expected []string }{ { - name: "externalHost is the only candidate", + name: "externalHost leads, request host still follows", hostHeader: "backend:8333", externalHost: "api.example.com:9000", + expected: []string{"api.example.com:9000", "backend:8333"}, + }, + { + name: "externalHost leads the forwarded candidates", + hostHeader: "backend:8333", + forwardedHost: "example.com", + forwardedPort: "9000", + externalHost: "api.example.com", + expected: []string{"api.example.com", "example.com:9000", "example.com"}, + }, + { + name: "externalHost equal to the request host is not repeated", + hostHeader: "api.example.com:9000", + externalHost: "api.example.com:9000", expected: []string{"api.example.com:9000"}, }, { diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index 704200f34..f9fd535f6 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -65,7 +65,7 @@ type S3ApiServerOption struct { Ip string // address advertised to the cluster; empty falls back to BindIp BindIp string GrpcPort int - ExternalUrl string // external URL clients use, for signature verification behind a reverse proxy + ExternalUrl string // external URL clients use, tried first during signature verification behind a reverse proxy DefaultFileMode uint32 // default file permission mode for S3 uploads (e.g. 0660, 0644) CacheSizeMB int64 // in-memory chunk cache capacity in MB for the shared ReaderCache; 0 disables MaxMB int32 // filer's -maxMB, read from the filer configuration at startup