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) } }