filer, s3: reuse the volume server's guarded remote-storage client builder (#10990)

* volume: build the guarded remote storage client through a shared helper

Fold the endpoint validation, credential check and rebinding-safe dialer
that FetchAndWriteNeedle applies before dialing a caller-supplied remote
storage endpoint into a single BuildGuardedRemoteStorageClient helper, so
other callers that dial the same endpoints can reuse it. No behavior
change on this path.

Claude-Session: https://claude.ai/code/session_01AiH1FU3rmshSbFFTbJpaZN

* filer: build the remote-mount stream client through the guarded helper

streamFromRemote serves a cold remote-only entry straight from its mounted
origin. Build its client through BuildGuardedRemoteStorageClient so the
same endpoint checks the volume server applies cover this read path too.

Claude-Session: https://claude.ai/code/session_01AiH1FU3rmshSbFFTbJpaZN

* s3: build the remote-mount stream client through the guarded helper

openRemoteStream serves a remote-mounted object straight from its origin
when the local read cannot. Build its client through the same guarded
helper so the endpoint checks apply here as well.

Claude-Session: https://claude.ai/code/session_01AiH1FU3rmshSbFFTbJpaZN
This commit is contained in:
Chris Lu
2026-08-27 16:25:56 -07:00
committed by GitHub
parent 28862c866e
commit 0b5fff2ccd
4 changed files with 74 additions and 24 deletions
+2 -1
View File
@@ -24,6 +24,7 @@ import (
"github.com/seaweedfs/seaweedfs/weed/pb/remote_pb"
"github.com/seaweedfs/seaweedfs/weed/remote_storage"
"github.com/seaweedfs/seaweedfs/weed/security"
weed_server "github.com/seaweedfs/seaweedfs/weed/server"
"github.com/seaweedfs/seaweedfs/weed/util"
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
@@ -3337,7 +3338,7 @@ func (s3a *S3ApiServer) openRemoteStream(ctx context.Context, bucket, object str
return nil, err
}
client, err := remote_storage.GetRemoteStorage(storageConf)
client, err := weed_server.BuildGuardedRemoteStorageClient(ctx, storageConf, false)
if err != nil {
return nil, err
}
+1 -1
View File
@@ -286,7 +286,7 @@ func (fs *FilerServer) streamFromRemote(ctx context.Context, dir, name string, o
if err != nil {
return nil, err
}
client, err := remote_storage.GetRemoteStorage(storageConf)
client, err := BuildGuardedRemoteStorageClient(ctx, storageConf, false)
if err != nil {
return nil, err
}
+38 -22
View File
@@ -350,6 +350,42 @@ func checkGcsCredentials(creds string) error {
return nil
}
// BuildGuardedRemoteStorageClient builds a remote storage client whose dial
// path is validated against the SSRF deny-list and pinned against DNS
// rebinding, unless allowUntrusted is set. It is the single builder for every
// caller that dials a caller-influenced RemoteConf endpoint: the volume
// FetchAndWriteNeedle write path and the filer and S3 gateway remote-mount read
// paths, which otherwise reach the same s3manager sink unguarded.
func BuildGuardedRemoteStorageClient(ctx context.Context, remoteConf *remote_pb.RemoteConf, allowUntrusted bool) (remote_storage.RemoteStorageClient, error) {
if !allowUntrusted {
if remoteConf.GetType() == "gcs" {
if credsErr := checkGcsCredentials(remoteConf.GetGcsGoogleApplicationCredentials()); credsErr != nil {
return nil, fmt.Errorf("reject remote credentials: %w", credsErr)
}
}
if endpoint, makeClient, ok := guardedRemoteClient(remoteConf); ok {
if validateErr := validateRemoteEndpoint(ctx, endpoint); validateErr != nil {
return nil, fmt.Errorf("reject remote endpoint: %w", validateErr)
}
// 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 SDK dials).
client, err := makeClient(newGuardedHTTPClient(endpoint))
if err != nil {
return nil, fmt.Errorf("get remote client: %w", err)
}
return client, nil
}
}
client, err := remote_storage.GetRemoteStorage(remoteConf)
if err != nil {
return nil, fmt.Errorf("get remote client: %w", err)
}
return client, nil
}
func (vs *VolumeServer) FetchAndWriteNeedle(ctx context.Context, req *volume_server_pb.FetchAndWriteNeedleRequest) (resp *volume_server_pb.FetchAndWriteNeedleResponse, err error) {
if err := vs.checkGrpcAdminAuth(ctx); err != nil {
return nil, err
@@ -366,29 +402,9 @@ func (vs *VolumeServer) FetchAndWriteNeedle(ctx context.Context, req *volume_ser
remoteConf := req.RemoteConf
if !vs.AllowUntrustedRemoteEndpoints && remoteConf.GetType() == "gcs" {
if credsErr := checkGcsCredentials(remoteConf.GetGcsGoogleApplicationCredentials()); credsErr != nil {
return nil, fmt.Errorf("reject remote credentials: %w", credsErr)
}
}
var client remote_storage.RemoteStorageClient
var getClientErr error
if endpoint, makeClient, ok := guardedRemoteClient(remoteConf); ok && !vs.AllowUntrustedRemoteEndpoints {
if validateErr := validateRemoteEndpoint(ctx, endpoint); validateErr != nil {
return nil, fmt.Errorf("reject remote endpoint: %w", validateErr)
}
// 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 SDK dials).
client, getClientErr = makeClient(newGuardedHTTPClient(endpoint))
} else {
client, getClientErr = remote_storage.GetRemoteStorage(remoteConf)
}
client, getClientErr := BuildGuardedRemoteStorageClient(ctx, remoteConf, vs.AllowUntrustedRemoteEndpoints)
if getClientErr != nil {
return nil, fmt.Errorf("get remote client: %w", getClientErr)
return nil, getClientErr
}
remoteStorageLocation := req.RemoteLocation
+33
View File
@@ -638,3 +638,36 @@ func TestGuardedReplicaDialerRebind(t *testing.T) {
t.Fatalf("private peer must be allowed by the replica dialer, got %v", perr)
}
}
// TestBuildGuardedRemoteStorageClient confirms the shared builder refuses a
// caller-influenced endpoint that resolves to a blocked address, and a gcs
// credentials path, while allowUntrusted falls back to the plain builder.
func TestBuildGuardedRemoteStorageClient(t *testing.T) {
loopbackS3 := &remote_pb.RemoteConf{
Name: "poc",
Type: "s3",
S3Endpoint: "http://127.0.0.1:8000",
S3AccessKey: "k",
S3SecretKey: "s",
S3Region: "us-east-1",
}
if _, err := BuildGuardedRemoteStorageClient(context.Background(), loopbackS3, false); err == nil {
t.Error("expected a loopback s3 endpoint to be rejected")
} else if !strings.Contains(err.Error(), "reject remote endpoint") {
t.Errorf("error = %v, want reject remote endpoint", err)
}
if _, err := BuildGuardedRemoteStorageClient(context.Background(), loopbackS3, true); err != nil {
t.Errorf("allowUntrusted should build the client: %v", err)
}
gcsPathCreds := &remote_pb.RemoteConf{
Name: "poc",
Type: "gcs",
GcsGoogleApplicationCredentials: "/etc/hostname",
}
if _, err := BuildGuardedRemoteStorageClient(context.Background(), gcsPathCreds, false); err == nil {
t.Error("expected a gcs credentials path to be rejected")
} else if !strings.Contains(err.Error(), "reject remote credentials") {
t.Errorf("error = %v, want reject remote credentials", err)
}
}