From 92c379e5b4a0296e213c777fc7b7b01730bec843 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Sun, 13 Sep 2026 14:43:45 -0700 Subject: [PATCH] filer: accept gcs credentials file paths in the guarded remote client builder (#11296) * filer: accept gcs credentials file paths in the guarded remote client builder checkGcsCredentials rejected all filesystem paths, so a gcs mount configured with remote.configure -gcs.appCredentialsFile (which stores a path in GcsGoogleApplicationCredentials) was rejected by BuildGuardedRemoteStorageClient with "gcs credentials must be inline JSON". This broke existing gcs mounts on the volume, filer, and s3 remote-mount read paths that use the guarded builder. Read and validate the file content instead of rejecting the path, mirroring what the gcs client itself does in MakeWithHTTPClient. A path that does not exist or does not contain valid gcs credentials is still rejected before any client is built. guardedRemoteClient now reads the file to extract the token exchange URL for the SSRF deny-list, so the rebinding-safe dialer still guards the token endpoint. * filer: resolve gcs credential paths and avoid leaking file existence loadGcsCredentialsContent passed the raw credentials string to os.ReadFile, so a documented ~/path (as written by remote.configure -gcs.appCredentialsFile=~/...) was rejected because os.ReadFile does not expand ~. It also wrapped the os.ReadFile error, which includes the file path, exposing file existence to a caller who planted a conf with an arbitrary path. Resolve the path with util.ResolvePath, matching the gcs client's own behavior in MakeWithHTTPClient. Return a generic sentinel error on read failure so the path is not reflected in the error message. The credential type validation still runs on the file content, so a path that does not contain valid gcs credentials is rejected before any client is built. --- weed/server/volume_grpc_remote.go | 43 ++++++++++++++++++++------ weed/server/volume_grpc_remote_test.go | 36 ++++++++++++++++++++- 2 files changed, 69 insertions(+), 10 deletions(-) diff --git a/weed/server/volume_grpc_remote.go b/weed/server/volume_grpc_remote.go index 4eedef097..8b32ccb66 100644 --- a/weed/server/volume_grpc_remote.go +++ b/weed/server/volume_grpc_remote.go @@ -2,10 +2,12 @@ package weed_server import ( "context" + "errors" "fmt" "net" "net/http" "net/url" + "os" "slices" "strings" "sync" @@ -21,6 +23,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/security" "github.com/seaweedfs/seaweedfs/weed/storage/needle" "github.com/seaweedfs/seaweedfs/weed/storage/types" + "github.com/seaweedfs/seaweedfs/weed/util" ) // lookupIPAddrFunc resolves a host to one or more IP addresses. It is a @@ -313,10 +316,12 @@ func guardedRemoteClient(remoteConf *remote_pb.RemoteConf) (endpoint string, mak // gcs reaches a fixed object host, but the token exchange goes wherever the // supplied credentials say, so guard that endpoint instead. if remoteConf.Type == "gcs" && remoteConf.GcsGoogleApplicationCredentials != "" { - if _, tokenURL, err := gcsremote.ParseInlineCredentials(remoteConf.GcsGoogleApplicationCredentials); err == nil { - return tokenURL, func(httpClient *http.Client) (remote_storage.RemoteStorageClient, error) { - return gcsremote.MakeWithHTTPClient(remoteConf, httpClient, gcsremote.StaticKeyCredentialTypes...) - }, true + if data, err := loadGcsCredentialsContent(remoteConf.GcsGoogleApplicationCredentials); err == nil { + if _, tokenURL, parseErr := gcsremote.ParseInlineCredentials(string(data)); parseErr == nil { + return tokenURL, func(httpClient *http.Client) (remote_storage.RemoteStorageClient, error) { + return gcsremote.MakeWithHTTPClient(remoteConf, httpClient, gcsremote.StaticKeyCredentialTypes...) + }, true + } } } return "", nil, false @@ -328,6 +333,27 @@ func gcsCredentialsArePath(creds string) bool { return creds != "" && !strings.HasPrefix(creds, "{") } +var errGcsCredentialsUnreadable = errors.New("gcs credentials file is not readable or does not contain valid credentials") + +// loadGcsCredentialsContent returns the credential JSON for a gcs credentials +// value, reading from disk when it is a filesystem path (as written by +// remote.configure -gcs.appCredentialsFile). This mirrors what the gcs client +// itself does in MakeWithHTTPClient, so the guard validates the same content +// the client will eventually load. +func loadGcsCredentialsContent(creds string) ([]byte, error) { + if creds == "" { + return nil, nil + } + if strings.HasPrefix(creds, "{") { + return []byte(creds), nil + } + data, err := os.ReadFile(util.ResolvePath(creds)) + if err != nil { + return nil, errGcsCredentialsUnreadable + } + return data, nil +} + // checkGcsCredentials rejects a caller-supplied gcs credentials value that // would make the SDK read from somewhere other than the credentials themselves, // so the request fails before any client is built. @@ -335,12 +361,11 @@ func checkGcsCredentials(creds string) error { if creds == "" { return nil } - // A filesystem path is read from disk by the SDK. Accept only inline JSON - // on the request; the server env var still supplies a path. - if gcsCredentialsArePath(creds) { - return fmt.Errorf("gcs credentials must be inline JSON") + data, err := loadGcsCredentialsContent(creds) + if err != nil { + return err } - credType, _, parseErr := gcsremote.ParseInlineCredentials(creds) + credType, _, parseErr := gcsremote.ParseInlineCredentials(string(data)) if parseErr != nil { return parseErr } diff --git a/weed/server/volume_grpc_remote_test.go b/weed/server/volume_grpc_remote_test.go index 0d3b2bf08..3347482f0 100644 --- a/weed/server/volume_grpc_remote_test.go +++ b/weed/server/volume_grpc_remote_test.go @@ -4,6 +4,8 @@ import ( "context" "errors" "net" + "os" + "path/filepath" "strings" "sync/atomic" "testing" @@ -666,8 +668,40 @@ func TestBuildGuardedRemoteStorageClient(t *testing.T) { GcsGoogleApplicationCredentials: "/etc/hostname", } if _, err := BuildGuardedRemoteStorageClient(context.Background(), gcsPathCreds, false); err == nil { - t.Error("expected a gcs credentials path to be rejected") + t.Error("expected a non-credentials file path to be rejected") } else if !strings.Contains(err.Error(), "reject remote credentials") { t.Errorf("error = %v, want reject remote credentials", err) + } else if strings.Contains(err.Error(), "/etc/hostname") { + t.Errorf("error must not leak the file path: %v", err) + } + + // A file path that points to valid GCS credentials should be accepted. + credsFile := filepath.Join(t.TempDir(), "service-account.json") + validCreds := `{"type":"service_account","token_uri":"https://oauth2.googleapis.com/token","client_email":"sa@example.iam.gserviceaccount.com","private_key":"-----BEGIN PRIVATE KEY-----\nMIIBVwIBADANBgkqhkiG9w0BAQEFAASCAUEwggE9AgEAAkEAxY\n-----END PRIVATE KEY-----\n","private_key_id":"key1"}` + if err := os.WriteFile(credsFile, []byte(validCreds), 0600); err != nil { + t.Fatalf("write creds file: %v", err) + } + gcsFileCreds := &remote_pb.RemoteConf{ + Name: "good", + Type: "gcs", + GcsGoogleApplicationCredentials: credsFile, + } + if err := checkGcsCredentials(credsFile); err != nil { + t.Errorf("valid gcs credentials file should pass: %v", err) + } + if _, err := BuildGuardedRemoteStorageClient(context.Background(), gcsFileCreds, false); err != nil { + t.Errorf("valid gcs credentials file should build: %v", err) + } + + // A nonexistent path must be rejected without leaking the path in the error. + gcsMissingCreds := &remote_pb.RemoteConf{ + Name: "missing", + Type: "gcs", + GcsGoogleApplicationCredentials: filepath.Join(t.TempDir(), "does-not-exist.json"), + } + if _, err := BuildGuardedRemoteStorageClient(context.Background(), gcsMissingCreds, false); err == nil { + t.Error("expected a nonexistent credentials file to be rejected") + } else if strings.Contains(err.Error(), "does-not-exist") { + t.Errorf("error must not leak the file path: %v", err) } }