Guard the gcs credential path in FetchAndWriteNeedle like the other backends (#10796)

* volume: accept only static-key gcs credentials on the fetch request

An inline credentials document of a federated type points the SDK at a url,
file or executable of the caller's choosing for the token exchange, so the
request-supplied value is no longer just a key.

* volume: guard the gcs token endpoint like the other remote endpoints

Inline credentials pick where the token request goes, so route the gcs client
through the same deny-list and rebinding-safe dialer used for S3 and azure.

* rust volume: pin that gcs has no credential-driven dial path

* volume: only check gcs credentials on a gcs remote conf

Only the gcs backend reads that field, so another backend carrying a stale
value should not fail the request.

* gcs: load credentials with the type the caller expects

The untyped loader is deprecated because it reads whatever the document
claims to be; callers handling credentials they do not control now name the
types they accept.
This commit is contained in:
Chris Lu
2026-08-17 16:40:56 -07:00
committed by GitHub
parent 9d8acbd244
commit 6fda8c67f3
5 changed files with 215 additions and 13 deletions
+17
View File
@@ -226,4 +226,21 @@ mod tests {
assert_eq!(s3_compatible_endpoint(&azure), None);
assert!(make_remote_storage_client(&azure).is_err());
}
#[test]
fn gcs_credentials_have_no_ssrf_path() {
// The Go volume server accepts only static-key gcs credentials and puts
// their token endpoint behind the SSRF guard, because the SDK dials
// whatever url, file or executable the credentials name. This server has
// no gcs backend, so make_remote_storage_client rejects the type before
// any credentials are parsed. Anyone adding one must carry both guards
// over with it.
let gcs = RemoteConf {
r#type: "gcs".to_string(),
gcs_google_application_credentials: r#"{"type":"external_account","credential_source":{"url":"http://169.254.169.254/"}}"#.to_string(),
..Default::default()
};
assert_eq!(s3_compatible_endpoint(&gcs), None);
assert!(make_remote_storage_client(&gcs).is_err());
}
}
+57 -4
View File
@@ -2,11 +2,14 @@ package gcs
import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
"os"
"reflect"
"slices"
"strings"
"time"
@@ -27,6 +30,34 @@ func init() {
remote_storage.RemoteStorageClientMakers["gcs"] = new(gcsRemoteStorageMaker)
}
// defaultTokenURL is where the SDK sends the token request when the credentials
// leave token_uri unset.
const defaultTokenURL = "https://oauth2.googleapis.com/token"
// StaticKeyCredentialTypes are the credential types that carry their own key
// material. Every other type tells the SDK to fetch the token from a url, file
// or executable named inside the credentials.
var StaticKeyCredentialTypes = []string{
string(google.ServiceAccount),
string(google.AuthorizedUser),
}
// ParseInlineCredentials reports the credential type of an inline credentials
// document and the token endpoint it makes the SDK dial.
func ParseInlineCredentials(creds string) (credType string, tokenURL string, err error) {
var doc struct {
Type string `json:"type"`
TokenURI string `json:"token_uri"`
}
if err := json.Unmarshal([]byte(creds), &doc); err != nil {
return "", "", fmt.Errorf("parse gcs credentials: %w", err)
}
if doc.TokenURI == "" {
return doc.Type, defaultTokenURL, nil
}
return doc.Type, doc.TokenURI, nil
}
type gcsRemoteStorageMaker struct{}
func (s gcsRemoteStorageMaker) HasBucket() bool {
@@ -34,10 +65,24 @@ func (s gcsRemoteStorageMaker) HasBucket() bool {
}
func (s gcsRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.RemoteStorageClient, error) {
return MakeWithHTTPClient(conf, nil)
}
// MakeWithHTTPClient builds a gcs client whose token exchange and object reads
// both go through the supplied *http.Client (or the SDK default when nil).
// Callers that need to pin the dial path against DNS rebinding pass a client
// whose transport has a guarded DialContext, mirroring the S3 backend. Callers
// handling credentials they do not control pass the types they accept.
func MakeWithHTTPClient(conf *remote_pb.RemoteConf, httpClient *http.Client, allowedTypes ...string) (remote_storage.RemoteStorageClient, error) {
client := &gcsRemoteStorageClient{
conf: conf,
}
ctx := context.Background()
if httpClient != nil {
ctx = context.WithValue(ctx, oauth2.HTTPClient, httpClient)
}
googleApplicationCredentials := conf.GcsGoogleApplicationCredentials
if googleApplicationCredentials == "" {
@@ -71,15 +116,23 @@ func (s gcsRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.
return nil, fmt.Errorf("failed to read credentials file %s: %w", googleApplicationCredentials, err)
}
}
creds, err := google.CredentialsFromJSON(context.Background(), data, storage.ScopeFullControl)
credType, _, parseErr := ParseInlineCredentials(string(data))
if parseErr != nil {
return nil, parseErr
}
if len(allowedTypes) > 0 && !slices.Contains(allowedTypes, credType) {
return nil, fmt.Errorf("gcs credential type %q is not accepted here", credType)
}
// Declaring the type keeps the SDK from reading the document as anything
// else; the untyped loader is deprecated for exactly that reason.
creds, err := google.CredentialsFromJSONWithType(ctx, data, google.CredentialsType(credType), storage.ScopeFullControl)
if err != nil {
return nil, fmt.Errorf("failed to parse credentials: %w", err)
}
httpClient := oauth2.NewClient(context.Background(), creds.TokenSource)
clientOpts = append(clientOpts, option.WithHTTPClient(httpClient), option.WithoutAuthentication())
clientOpts = append(clientOpts, option.WithHTTPClient(oauth2.NewClient(ctx, creds.TokenSource)), option.WithoutAuthentication())
}
c, err := storage.NewClient(context.Background(), clientOpts...)
c, err := storage.NewClient(ctx, clientOpts...)
if err != nil {
return nil, fmt.Errorf("failed to create client: %w", err)
}
@@ -3,6 +3,7 @@ package gcs
import (
"testing"
"github.com/seaweedfs/seaweedfs/weed/pb/remote_pb"
"github.com/seaweedfs/seaweedfs/weed/remote_storage"
"github.com/stretchr/testify/require"
)
@@ -15,3 +16,31 @@ func TestGCSErrRemoteObjectNotFoundIsAccessible(t *testing.T) {
require.Error(t, remote_storage.ErrRemoteObjectNotFound)
require.Equal(t, "remote object not found", remote_storage.ErrRemoteObjectNotFound.Error())
}
// TestMakeWithHTTPClientAllowedTypes covers the restriction a caller applies to
// credentials it does not control: a federated document never reaches the SDK,
// while an unrestricted caller keeps loading whatever the operator configured.
func TestMakeWithHTTPClientAllowedTypes(t *testing.T) {
federated := `{"type":"external_account","audience":"a","subject_token_type":"t","token_url":"http://127.0.0.1:9/v1/token","credential_source":{"url":"http://169.254.169.254/"}}`
conf := &remote_pb.RemoteConf{Type: "gcs", GcsGoogleApplicationCredentials: federated}
_, err := MakeWithHTTPClient(conf, nil, StaticKeyCredentialTypes...)
require.ErrorContains(t, err, `"external_account" is not accepted`)
_, err = MakeWithHTTPClient(conf, nil)
require.NoError(t, err)
}
func TestParseInlineCredentials(t *testing.T) {
credType, tokenURL, err := ParseInlineCredentials(`{"type":"service_account"}`)
require.NoError(t, err)
require.Equal(t, "service_account", credType)
require.Equal(t, defaultTokenURL, tokenURL)
_, tokenURL, err = ParseInlineCredentials(`{"type":"service_account","token_uri":"https://example.com/t"}`)
require.NoError(t, err)
require.Equal(t, "https://example.com/t", tokenURL)
_, _, err = ParseInlineCredentials(`not json`)
require.Error(t, err)
}
+40 -9
View File
@@ -6,6 +6,7 @@ import (
"net"
"net/http"
"net/url"
"slices"
"strings"
"sync"
"time"
@@ -15,6 +16,7 @@ import (
"github.com/seaweedfs/seaweedfs/weed/pb/volume_server_pb"
"github.com/seaweedfs/seaweedfs/weed/remote_storage"
azureremote "github.com/seaweedfs/seaweedfs/weed/remote_storage/azure"
gcsremote "github.com/seaweedfs/seaweedfs/weed/remote_storage/gcs"
s3remote "github.com/seaweedfs/seaweedfs/weed/remote_storage/s3"
"github.com/seaweedfs/seaweedfs/weed/security"
"github.com/seaweedfs/seaweedfs/weed/storage/needle"
@@ -290,9 +292,10 @@ func newGuardedHTTPClientPolicy(endpoint string, allowPrivate bool) *http.Client
// guardedRemoteClient reports the caller-supplied endpoint a backend dials
// directly and a constructor that routes through the given HTTP client, or
// ok=false for backends that only reach a fixed provider host. The S3-SDK
// family and azure (once AzureEndpoint is set) both honor an attacker-supplied
// endpoint, so both must pass the SSRF deny-list and rebinding-safe dialer.
// ok=false when nothing in the conf steers a destination. The S3-SDK family,
// azure (once AzureEndpoint is set) and the gcs token exchange all honor a
// caller-supplied endpoint, so each must pass the SSRF deny-list and the
// rebinding-safe dialer.
func guardedRemoteClient(remoteConf *remote_pb.RemoteConf) (endpoint string, makeClient func(*http.Client) (remote_storage.RemoteStorageClient, error), ok bool) {
if remoteConf == nil {
return "", nil, false
@@ -307,6 +310,15 @@ func guardedRemoteClient(remoteConf *remote_pb.RemoteConf) (endpoint string, mak
return azureremote.MakeWithHTTPClient(remoteConf, httpClient)
}, true
}
// 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
}
}
return "", nil, false
}
@@ -316,6 +328,28 @@ func gcsCredentialsArePath(creds string) bool {
return creds != "" && !strings.HasPrefix(creds, "{")
}
// 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.
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")
}
credType, _, parseErr := gcsremote.ParseInlineCredentials(creds)
if parseErr != nil {
return parseErr
}
if !slices.Contains(gcsremote.StaticKeyCredentialTypes, credType) {
return fmt.Errorf("gcs credential type %q is not accepted here", credType)
}
return 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
@@ -332,12 +366,9 @@ func (vs *VolumeServer) FetchAndWriteNeedle(ctx context.Context, req *volume_ser
remoteConf := req.RemoteConf
if !vs.AllowUntrustedRemoteEndpoints && remoteConf != nil {
// A gcs credentials value that is 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(remoteConf.GetGcsGoogleApplicationCredentials()) {
return nil, fmt.Errorf("reject remote credentials: gcs credentials must be inline JSON")
if !vs.AllowUntrustedRemoteEndpoints && remoteConf.GetType() == "gcs" {
if credsErr := checkGcsCredentials(remoteConf.GetGcsGoogleApplicationCredentials()); credsErr != nil {
return nil, fmt.Errorf("reject remote credentials: %w", credsErr)
}
}
+72
View File
@@ -503,6 +503,78 @@ func TestGcsCredentialsArePath(t *testing.T) {
}
}
// TestGuardedRemoteClientGuardsGcsTokenURL confirms the token endpoint named by
// inline gcs credentials is the endpoint the guard validates, so a loopback
// token_uri is refused while the Google default passes.
func TestGuardedRemoteClientGuardsGcsTokenURL(t *testing.T) {
originalLookup := lookupIPAddrFunc
t.Cleanup(func() { lookupIPAddrFunc = originalLookup })
lookupIPAddrFunc = stubLookup(t, map[string][]net.IP{
"oauth2.googleapis.com": {net.ParseIP("142.250.72.10")},
})
endpoint, makeClient, ok := guardedRemoteClient(&remote_pb.RemoteConf{
Type: "gcs",
GcsGoogleApplicationCredentials: `{"type":"service_account","token_uri":"http://127.0.0.1:9/token"}`,
})
if !ok {
t.Fatal("gcs conf with inline credentials should be guarded")
}
if endpoint != "http://127.0.0.1:9/token" {
t.Errorf("endpoint = %q, want the credential token_uri", endpoint)
}
if err := validateRemoteEndpoint(context.Background(), endpoint); err == nil {
t.Error("expected the loopback token endpoint to be rejected")
}
if makeClient == nil {
t.Error("expected a constructor")
}
endpoint, _, ok = guardedRemoteClient(&remote_pb.RemoteConf{
Type: "gcs",
GcsGoogleApplicationCredentials: `{"type":"service_account"}`,
})
if !ok {
t.Fatal("gcs conf with inline credentials should be guarded")
}
if err := validateRemoteEndpoint(context.Background(), endpoint); err != nil {
t.Errorf("default token endpoint %q should pass: %v", endpoint, err)
}
}
// TestCheckGcsCredentials confirms only inline credentials that carry their own
// key material are accepted. The federated types name a url, file or executable
// that the SDK reads the token from, none of which the endpoint guard sees.
func TestCheckGcsCredentials(t *testing.T) {
rejected := []string{
"/etc/hostname",
"~/creds.json",
`{`,
`{}`,
`{"type":"external_account","token_url":"http://127.0.0.1:9/v1/token","credential_source":{"url":"http://169.254.169.254/latest/meta-data/"}}`,
`{"type":"external_account","token_url":"http://127.0.0.1:9/v1/token","credential_source":{"file":"/etc/shadow"}}`,
`{"type":"external_account","credential_source":{"executable":{"command":"/bin/sh"}}}`,
`{"type":"external_account_authorized_user","token_url":"http://127.0.0.1:9/v1/token"}`,
`{"type":"impersonated_service_account","service_account_impersonation_url":"http://127.0.0.1:9/x"}`,
}
for _, creds := range rejected {
if err := checkGcsCredentials(creds); err == nil {
t.Errorf("expected %q to be rejected", creds)
}
}
accepted := []string{
"",
`{"type":"service_account","client_email":"a@b.com","private_key":"k"}`,
`{"type":"service_account","token_uri":"https://oauth2.googleapis.com/token"}`,
`{"type":"authorized_user","refresh_token":"r"}`,
}
for _, creds := range accepted {
if err := checkGcsCredentials(creds); err != nil {
t.Errorf("expected %q to be accepted, got %v", creds, err)
}
}
}
// TestGuardedDialerLiteralBlocked confirms that a literal blocked IP target
// is refused without any DNS lookup.
func TestGuardedDialerLiteralBlocked(t *testing.T) {