diff --git a/seaweed-volume/src/remote_storage/mod.rs b/seaweed-volume/src/remote_storage/mod.rs index 49be142d6..459b4eba5 100644 --- a/seaweed-volume/src/remote_storage/mod.rs +++ b/seaweed-volume/src/remote_storage/mod.rs @@ -211,4 +211,19 @@ mod tests { }; assert_eq!(s3_compatible_endpoint(&gcs), None); } + + #[test] + fn azure_endpoint_has_no_ssrf_path() { + // The Go volume server guards the caller-supplied azure endpoint against + // SSRF. This server has no azure backend, so there is nothing to dial: + // azure is not S3-compatible (the endpoint guard does not apply) and + // make_remote_storage_client rejects the type before building a client. + let azure = RemoteConf { + r#type: "azure".to_string(), + azure_endpoint: "https://169.254.169.254/".to_string(), + ..Default::default() + }; + assert_eq!(s3_compatible_endpoint(&azure), None); + assert!(make_remote_storage_client(&azure).is_err()); + } } diff --git a/weed/remote_storage/azure/azure_credentials.go b/weed/remote_storage/azure/azure_credentials.go index d749b4136..a163186b5 100644 --- a/weed/remote_storage/azure/azure_credentials.go +++ b/weed/remote_storage/azure/azure_credentials.go @@ -2,6 +2,7 @@ package azure import ( "fmt" + "net/http" "net/url" "os" "regexp" @@ -27,7 +28,10 @@ var validAzureAccountName = regexp.MustCompile(`^[a-z0-9]{3,24}$`) // // endpoint is the blob service URL, for accounts outside the public cloud. An // empty endpoint derives the public one from accountName. -func NewAzBlobClient(accountName, accountKey, clientID, endpoint string) (*azblob.Client, error) { +// +// A non-nil httpClient overrides the SDK transport; the volume server passes one +// whose dialer pins the validated endpoint against DNS rebinding. +func NewAzBlobClient(accountName, accountKey, clientID, endpoint string, httpClient *http.Client) (*azblob.Client, error) { if accountName == "" { return nil, fmt.Errorf("azure account name is required") @@ -41,13 +45,18 @@ func NewAzBlobClient(accountName, accountKey, clientID, endpoint string) (*azblo return nil, err } + options := DefaultAzBlobClientOptions() + if httpClient != nil { + options.Transport = httpClient + } + if accountKey == "" { credential, err := newAzureTokenCredential(clientID) if err != nil { return nil, fmt.Errorf("failed to create Azure Entra ID credential for account %s: %w", accountName, err) } glog.V(1).Infof("azure %s: authenticating with Entra ID", accountName) - client, err := azblob.NewClient(serviceURL, credential, DefaultAzBlobClientOptions()) + client, err := azblob.NewClient(serviceURL, credential, options) if err != nil { return nil, fmt.Errorf("failed to create Azure client: %w", err) } @@ -58,7 +67,7 @@ func NewAzBlobClient(accountName, accountKey, clientID, endpoint string) (*azblo if err != nil { return nil, fmt.Errorf("failed to create Azure credential with account name:%s: %w", accountName, err) } - client, err := azblob.NewClientWithSharedKeyCredential(serviceURL, credential, DefaultAzBlobClientOptions()) + client, err := azblob.NewClientWithSharedKeyCredential(serviceURL, credential, options) if err != nil { return nil, fmt.Errorf("failed to create Azure client: %w", err) } diff --git a/weed/remote_storage/azure/azure_credentials_test.go b/weed/remote_storage/azure/azure_credentials_test.go index 6f63a9155..48206ac65 100644 --- a/weed/remote_storage/azure/azure_credentials_test.go +++ b/weed/remote_storage/azure/azure_credentials_test.go @@ -6,21 +6,21 @@ import ( ) func TestNewAzBlobClientRequiresAccountName(t *testing.T) { - if _, err := NewAzBlobClient("", "aW52YWxpZGtleQ==", "", ""); err == nil { + if _, err := NewAzBlobClient("", "aW52YWxpZGtleQ==", "", "", nil); err == nil { t.Error("expected an error without an account name") } } func TestNewAzBlobClientRejectsMalformedAccountName(t *testing.T) { for _, accountName := range []string{"ab", "TestAccount", "test-account", "evil.com/x", "evil@host.com", "account?x=1"} { - if _, err := NewAzBlobClient(accountName, "", "", ""); err == nil { + if _, err := NewAzBlobClient(accountName, "", "", "", nil); err == nil { t.Errorf("expected an error for account name %q", accountName) } } } func TestNewAzBlobClientSharedKey(t *testing.T) { - client, err := NewAzBlobClient("testaccount", "aW52YWxpZGtleQ==", "", "") + client, err := NewAzBlobClient("testaccount", "aW52YWxpZGtleQ==", "", "", nil) if err != nil { t.Fatalf("failed to create a shared key client: %v", err) } @@ -30,7 +30,7 @@ func TestNewAzBlobClientSharedKey(t *testing.T) { } func TestNewAzBlobClientSharedKeyRejectsMalformedKey(t *testing.T) { - if _, err := NewAzBlobClient("testaccount", "not base64", "", ""); err == nil { + if _, err := NewAzBlobClient("testaccount", "not base64", "", "", nil); err == nil { t.Error("expected an error with a malformed account key") } } @@ -62,7 +62,7 @@ func TestAzureServiceURL(t *testing.T) { // no account key: authenticate through the Entra ID chain instead func TestNewAzBlobClientEntraID(t *testing.T) { - client, err := NewAzBlobClient("testaccount", "", "", "") + client, err := NewAzBlobClient("testaccount", "", "", "", nil) if err != nil { t.Fatalf("failed to create an Entra ID client: %v", err) } @@ -75,7 +75,7 @@ func TestNewAzBlobClientWorkloadIdentity(t *testing.T) { t.Setenv("AZURE_FEDERATED_TOKEN_FILE", filepath.Join(t.TempDir(), "token")) t.Setenv("AZURE_TENANT_ID", "00000000-0000-0000-0000-000000000000") - client, err := NewAzBlobClient("testaccount", "", "11111111-1111-1111-1111-111111111111", "") + client, err := NewAzBlobClient("testaccount", "", "11111111-1111-1111-1111-111111111111", "", nil) if err != nil { t.Fatalf("failed to create a workload identity client: %v", err) } diff --git a/weed/remote_storage/azure/azure_storage_client.go b/weed/remote_storage/azure/azure_storage_client.go index 5e9495ab2..18c7ed47d 100644 --- a/weed/remote_storage/azure/azure_storage_client.go +++ b/weed/remote_storage/azure/azure_storage_client.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "math" + "net/http" "os" "reflect" "regexp" @@ -116,6 +117,14 @@ func (s azureRemoteStorageMaker) HasBucket() bool { } func (s azureRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storage.RemoteStorageClient, error) { + return MakeWithHTTPClient(conf, nil) +} + +// MakeWithHTTPClient builds an azure client using the supplied *http.Client for +// its transport (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. +func MakeWithHTTPClient(conf *remote_pb.RemoteConf, httpClient *http.Client) (remote_storage.RemoteStorageClient, error) { client := &azureRemoteStorageClient{ conf: conf, @@ -126,7 +135,7 @@ func (s azureRemoteStorageMaker) Make(conf *remote_pb.RemoteConf) (remote_storag return nil, fmt.Errorf("neither azure_account_name nor the AZURE_STORAGE_ACCOUNT environment variable is set") } - azClient, err := NewAzBlobClient(accountName, accountKey, conf.AzureClientId, conf.AzureEndpoint) + azClient, err := NewAzBlobClient(accountName, accountKey, conf.AzureClientId, conf.AzureEndpoint, httpClient) if err != nil { return nil, err } diff --git a/weed/replication/sink/azuresink/azure_sink.go b/weed/replication/sink/azuresink/azure_sink.go index 6688483a4..9869d6f34 100644 --- a/weed/replication/sink/azuresink/azure_sink.go +++ b/weed/replication/sink/azuresink/azure_sink.go @@ -66,7 +66,7 @@ func (g *AzureSink) initialize(accountName, accountKey, clientID, endpoint, cont g.container = container g.dir = dir - client, err := azure.NewAzBlobClient(accountName, accountKey, clientID, endpoint) + client, err := azure.NewAzBlobClient(accountName, accountKey, clientID, endpoint, nil) if err != nil { return err } diff --git a/weed/server/volume_grpc_remote.go b/weed/server/volume_grpc_remote.go index 472df49d1..033c3c532 100644 --- a/weed/server/volume_grpc_remote.go +++ b/weed/server/volume_grpc_remote.go @@ -11,8 +11,10 @@ import ( "time" "github.com/seaweedfs/seaweedfs/weed/operation" + "github.com/seaweedfs/seaweedfs/weed/pb/remote_pb" "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" s3remote "github.com/seaweedfs/seaweedfs/weed/remote_storage/s3" "github.com/seaweedfs/seaweedfs/weed/security" "github.com/seaweedfs/seaweedfs/weed/storage/needle" @@ -218,6 +220,28 @@ func newGuardedHTTPClient(endpoint string) *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. +func guardedRemoteClient(remoteConf *remote_pb.RemoteConf) (endpoint string, makeClient func(*http.Client) (remote_storage.RemoteStorageClient, error), ok bool) { + if remoteConf == nil { + return "", nil, false + } + if ep, isS3 := s3remote.S3CompatibleEndpoint(remoteConf); isS3 { + return ep, func(httpClient *http.Client) (remote_storage.RemoteStorageClient, error) { + return s3remote.MakeWithHTTPClient(remoteConf, httpClient) + }, true + } + if remoteConf.Type == "azure" && remoteConf.AzureEndpoint != "" { + return remoteConf.AzureEndpoint, func(httpClient *http.Client) (remote_storage.RemoteStorageClient, error) { + return azureremote.MakeWithHTTPClient(remoteConf, httpClient) + }, true + } + return "", nil, false +} + 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 @@ -234,19 +258,9 @@ func (vs *VolumeServer) FetchAndWriteNeedle(ctx context.Context, req *volume_ser remoteConf := req.RemoteConf - // Every S3-SDK-backed backend (s3, wasabi, b2, storj, contabo, tencent, - // aliyun, baidu, filebase) dials this caller-supplied endpoint directly, - // so the guard must cover all of them, not just type "s3". Other backends - // (gcs, azure, ...) authenticate against their own SDKs and don't accept - // an attacker-controlled host. - endpoint, isS3Compatible := "", false - if remoteConf != nil { - endpoint, isS3Compatible = s3remote.S3CompatibleEndpoint(remoteConf) - } - var client remote_storage.RemoteStorageClient var getClientErr error - if !vs.AllowUntrustedRemoteEndpoints && isS3Compatible { + 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) } @@ -254,8 +268,8 @@ func (vs *VolumeServer) FetchAndWriteNeedle(ctx context.Context, req *volume_ser // 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 AWS SDK dials). - client, getClientErr = s3remote.MakeWithHTTPClient(remoteConf, newGuardedHTTPClient(endpoint)) + // when the SDK dials). + client, getClientErr = makeClient(newGuardedHTTPClient(endpoint)) } else { client, getClientErr = remote_storage.GetRemoteStorage(remoteConf) } diff --git a/weed/server/volume_grpc_remote_test.go b/weed/server/volume_grpc_remote_test.go index 141d2eb9c..ebe7445d9 100644 --- a/weed/server/volume_grpc_remote_test.go +++ b/weed/server/volume_grpc_remote_test.go @@ -342,6 +342,76 @@ func TestRemoteEndpointGuardCoversS3CompatibleSiblings(t *testing.T) { } } +// TestRemoteEndpointGuardCoversAzure confirms the SSRF guard reaches the azure +// backend, which dials a caller-supplied AzureEndpoint. It replays the two +// steps FetchAndWriteNeedle performs before building the client: resolve the +// endpoint the type would dial via guardedRemoteClient, then validate it. An +// azure conf pointed at an internal address must be rejected. +func TestRemoteEndpointGuardCoversAzure(t *testing.T) { + cases := []struct { + name string + conf *remote_pb.RemoteConf + wantSub string + }{ + {"imds", &remote_pb.RemoteConf{Type: "azure", AzureEndpoint: "https://169.254.169.254/"}, "metadata"}, + {"loopback", &remote_pb.RemoteConf{Type: "azure", AzureEndpoint: "https://127.0.0.1/"}, "loopback"}, + {"private", &remote_pb.RemoteConf{Type: "azure", AzureEndpoint: "https://10.0.0.5/"}, "private"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + endpoint, _, ok := guardedRemoteClient(tc.conf) + if !ok { + t.Fatalf("azure endpoint %q not guarded, the SSRF check would be skipped", tc.conf.AzureEndpoint) + } + err := validateRemoteEndpoint(context.Background(), endpoint) + if err == nil { + t.Fatalf("expected endpoint %q to be rejected", endpoint) + } + if !strings.Contains(err.Error(), tc.wantSub) { + t.Fatalf("error %q missing %q", err, tc.wantSub) + } + }) + } +} + +// TestGuardedRemoteClientSkipsFixedHostBackends confirms backends that only +// reach a fixed provider host bypass the endpoint guard: azure with no explicit +// endpoint (public cloud, host derived from the account) and unrelated types. +func TestGuardedRemoteClientSkipsFixedHostBackends(t *testing.T) { + for _, conf := range []*remote_pb.RemoteConf{ + {Type: "azure", AzureAccountName: "acct"}, + {Type: "gcs"}, + nil, + } { + if _, _, ok := guardedRemoteClient(conf); ok { + t.Errorf("conf %+v should not be guarded", conf) + } + } +} + +// TestGuardedRemoteClientAzureBuildsGuardedClient exercises the whole azure +// path: a public endpoint passes validation and the constructor builds a client +// through the guarded HTTP transport. +func TestGuardedRemoteClientAzureBuildsGuardedClient(t *testing.T) { + conf := &remote_pb.RemoteConf{ + Type: "azure", + AzureAccountName: "testaccount", + AzureAccountKey: "aW52YWxpZGtleQ==", + AzureEndpoint: "https://testaccount.blob.core.usgovcloudapi.net/", + } + endpoint, makeClient, ok := guardedRemoteClient(conf) + if !ok { + t.Fatal("azure with an endpoint should be guarded") + } + client, err := makeClient(newGuardedHTTPClient(endpoint)) + if err != nil { + t.Fatalf("build guarded azure client: %v", err) + } + if client == nil { + t.Fatal("expected a client") + } +} + // TestGuardedDialerLiteralBlocked confirms that a literal blocked IP target // is refused without any DNS lookup. func TestGuardedDialerLiteralBlocked(t *testing.T) {