diff --git a/sdk/storage/azblob/CHANGELOG.md b/sdk/storage/azblob/CHANGELOG.md index 0d363352c48c..f3e61a635b7c 100644 --- a/sdk/storage/azblob/CHANGELOG.md +++ b/sdk/storage/azblob/CHANGELOG.md @@ -11,6 +11,8 @@ ### Bugs Fixed +* Block `SharedKeyCredential` authentication mode for non TLS protected endpoints. Fixes [#21841](https://github.com/Azure/azure-sdk-for-go/issues/21841). + ### Other Changes ## 1.2.1 (2023-12-13) diff --git a/sdk/storage/azblob/assets.json b/sdk/storage/azblob/assets.json index a2a2f83f8d49..df7d66f02108 100644 --- a/sdk/storage/azblob/assets.json +++ b/sdk/storage/azblob/assets.json @@ -2,5 +2,5 @@ "AssetsRepo": "Azure/azure-sdk-assets", "AssetsRepoPrefixPath": "go", "TagPrefix": "go/storage/azblob", - "Tag": "go/storage/azblob_ceb9b7d6b4" + "Tag": "go/storage/azblob_9f40a5a13d" } diff --git a/sdk/storage/azblob/internal/exported/shared_key_credential.go b/sdk/storage/azblob/internal/exported/shared_key_credential.go index bd0bd5e260db..e4b076601f4e 100644 --- a/sdk/storage/azblob/internal/exported/shared_key_credential.go +++ b/sdk/storage/azblob/internal/exported/shared_key_credential.go @@ -11,7 +11,9 @@ import ( "crypto/hmac" "crypto/sha256" "encoding/base64" + "errors" "fmt" + "github.com/Azure/azure-sdk-for-go/sdk/internal/errorinfo" "net/http" "net/url" "sort" @@ -195,6 +197,17 @@ func NewSharedKeyCredPolicy(cred *SharedKeyCredential) *SharedKeyCredPolicy { } func (s *SharedKeyCredPolicy) Do(req *policy.Request) (*http.Response, error) { + // skip adding the authorization header if no SharedKeyCredential was provided. + // this prevents a panic that might be hard to diagnose and allows testing + // against http endpoints that don't require authentication. + if s.cred == nil { + return req.Next() + } + + if err := checkHTTPSForAuth(req); err != nil { + return nil, err + } + if d := getHeader(shared.HeaderXmsDate, req.Raw().Header); d == "" { req.Raw().Header.Set(shared.HeaderXmsDate, time.Now().UTC().Format(http.TimeFormat)) } @@ -216,3 +229,10 @@ func (s *SharedKeyCredPolicy) Do(req *policy.Request) (*http.Response, error) { } return response, err } + +func checkHTTPSForAuth(req *policy.Request) error { + if strings.ToLower(req.Raw().URL.Scheme) != "https" { + return errorinfo.NonRetriableError(errors.New("authenticated requests are not permitted for non TLS protected (https) endpoints")) + } + return nil +} diff --git a/sdk/storage/azblob/service/client_test.go b/sdk/storage/azblob/service/client_test.go index 0449197a4013..bac044016dbf 100644 --- a/sdk/storage/azblob/service/client_test.go +++ b/sdk/storage/azblob/service/client_test.go @@ -1810,6 +1810,35 @@ func (s *ServiceUnrecordedTestsSuite) TestServiceBlobBatchErrors() { _require.Error(err) } +func (s *ServiceRecordedTestsSuite) TestServiceClientRejectHTTP() { + _require := require.New(s.T()) + + cred, err := testcommon.GetGenericSharedKeyCredential(testcommon.TestAccountDefault) + _require.NoError(err) + + svcClient, err := service.NewClientWithSharedKeyCredential("http://"+cred.AccountName()+".blob.core.windows.net/", cred, nil) + _require.NoError(err) + + _, err = svcClient.GetProperties(context.Background(), nil) + _require.Error(err) +} + +func (s *ServiceRecordedTestsSuite) TestServiceClientWithNilSharedKey() { + _require := require.New(s.T()) + + accountName, _ := testcommon.GetGenericAccountInfo(testcommon.TestAccountDefault) + _require.Greater(len(accountName), 0) + + options := &service.ClientOptions{} + testcommon.SetClientOptions(s.T(), &options.ClientOptions) + svcClient, err := service.NewClientWithSharedKeyCredential("https://"+accountName+".blob.core.windows.net/", nil, options) + _require.NoError(err) + + _, err = svcClient.GetProperties(context.Background(), nil) + _require.Error(err) + testcommon.ValidateBlobErrorCode(_require, err, bloberror.NoAuthenticationInformation) +} + func (s *ServiceRecordedTestsSuite) TestServiceClientDefaultAudience() { _require := require.New(s.T()) testName := s.T().Name() diff --git a/sdk/storage/azdatalake/CHANGELOG.md b/sdk/storage/azdatalake/CHANGELOG.md index 1dd1454ebc94..40d907997e00 100644 --- a/sdk/storage/azdatalake/CHANGELOG.md +++ b/sdk/storage/azdatalake/CHANGELOG.md @@ -12,8 +12,11 @@ ### Bugs Fixed +* Block `SharedKeyCredential` authentication mode for non TLS protected endpoints. Fixes [#21841](https://github.com/Azure/azure-sdk-for-go/issues/21841). + ### Other Changes * Updated version of azblob to 1.2.1 +* Updated azcore version to `1.9.1` and azidentity version to `1.4.0`. ## 1.0.0 (2023-10-18) diff --git a/sdk/storage/azdatalake/assets.json b/sdk/storage/azdatalake/assets.json index 877057ad3692..626d6568abd4 100644 --- a/sdk/storage/azdatalake/assets.json +++ b/sdk/storage/azdatalake/assets.json @@ -2,5 +2,5 @@ "AssetsRepo": "Azure/azure-sdk-assets", "AssetsRepoPrefixPath": "go", "TagPrefix": "go/storage/azdatalake", - "Tag": "go/storage/azdatalake_abd1896b54" -} \ No newline at end of file + "Tag": "go/storage/azdatalake_3ae5e1441b" +} diff --git a/sdk/storage/azdatalake/internal/exported/shared_key_credential.go b/sdk/storage/azdatalake/internal/exported/shared_key_credential.go index e75b29f0ad8b..bf0c728faf53 100644 --- a/sdk/storage/azdatalake/internal/exported/shared_key_credential.go +++ b/sdk/storage/azdatalake/internal/exported/shared_key_credential.go @@ -11,7 +11,9 @@ import ( "crypto/hmac" "crypto/sha256" "encoding/base64" + "errors" "fmt" + "github.com/Azure/azure-sdk-for-go/sdk/internal/errorinfo" "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob" "net/http" "net/url" @@ -50,6 +52,9 @@ func (c *SharedKeyCredential) AccountName() string { } func ConvertToBlobSharedKey(c *SharedKeyCredential) (*azblob.SharedKeyCredential, error) { + if c == nil { + return nil, errors.New("SharedKeyCredential cannot be nil") + } cred, err := azblob.NewSharedKeyCredential(c.accountName, c.accountKeyString) if err != nil { return nil, err @@ -205,6 +210,17 @@ func NewSharedKeyCredPolicy(cred *SharedKeyCredential) *SharedKeyCredPolicy { } func (s *SharedKeyCredPolicy) Do(req *policy.Request) (*http.Response, error) { + // skip adding the authorization header if no SharedKeyCredential was provided. + // this prevents a panic that might be hard to diagnose and allows testing + // against http endpoints that don't require authentication. + if s.cred == nil { + return req.Next() + } + + if err := checkHTTPSForAuth(req); err != nil { + return nil, err + } + if d := getHeader(shared.HeaderXmsDate, req.Raw().Header); d == "" { req.Raw().Header.Set(shared.HeaderXmsDate, time.Now().UTC().Format(http.TimeFormat)) } @@ -226,3 +242,11 @@ func (s *SharedKeyCredPolicy) Do(req *policy.Request) (*http.Response, error) { } return response, err } + +// TODO: update the azblob dependency having checks for rejecting URLs that are not HTTPS +func checkHTTPSForAuth(req *policy.Request) error { + if strings.ToLower(req.Raw().URL.Scheme) != "https" { + return errorinfo.NonRetriableError(errors.New("authenticated requests are not permitted for non TLS protected (https) endpoints")) + } + return nil +} diff --git a/sdk/storage/azdatalake/service/client_test.go b/sdk/storage/azdatalake/service/client_test.go index a8a0f8c6f196..8720bba2980f 100644 --- a/sdk/storage/azdatalake/service/client_test.go +++ b/sdk/storage/azdatalake/service/client_test.go @@ -800,3 +800,33 @@ func (s *ServiceRecordedTestsSuite) TestAccountListFilesystemsEmptyPrefix() { } _require.GreaterOrEqual(count, 2) } + +func (s *ServiceRecordedTestsSuite) TestServiceClientRejectHTTP() { + _require := require.New(s.T()) + testName := s.T().Name() + + cred, err := testcommon.GetGenericSharedKeyCredential(testcommon.TestAccountDatalake) + _require.NoError(err) + + svcClient, err := service.NewClientWithSharedKeyCredential("http://"+cred.AccountName()+".dfs.core.windows.net/", cred, nil) + _require.NoError(err) + + fsName := testcommon.GenerateFileSystemName(testName) + fileName := testcommon.GenerateFileName(testName) + fileClient := svcClient.NewFileSystemClient(fsName).NewFileClient(fileName) + _require.Equal(fileClient.DFSURL(), "http://"+cred.AccountName()+".dfs.core.windows.net/"+fsName+"/"+fileName) + + _, err = fileClient.Create(context.Background(), nil) + _require.Error(err) +} + +func (s *ServiceRecordedTestsSuite) TestServiceClientWithNilSharedKey() { + _require := require.New(s.T()) + + accountName, _ := testcommon.GetGenericAccountInfo(testcommon.TestAccountDatalake) + _require.Greater(len(accountName), 0) + + svcClient, err := service.NewClientWithSharedKeyCredential("http://"+accountName+".dfs.core.windows.net/", nil, nil) + _require.Error(err) + _require.Nil(svcClient) +} diff --git a/sdk/storage/azfile/CHANGELOG.md b/sdk/storage/azfile/CHANGELOG.md index 973dbcf87a93..adc678058e38 100644 --- a/sdk/storage/azfile/CHANGELOG.md +++ b/sdk/storage/azfile/CHANGELOG.md @@ -12,6 +12,7 @@ ### Bugs Fixed +* Block `SharedKeyCredential` authentication mode for non TLS protected endpoints. Fixes [#21841](https://github.com/Azure/azure-sdk-for-go/issues/21841). * Fixed a bug where `UploadRangeFromURL` using OAuth was returning error. ### Other Changes diff --git a/sdk/storage/azfile/assets.json b/sdk/storage/azfile/assets.json index 4c4caf344b81..0c84c78afe62 100644 --- a/sdk/storage/azfile/assets.json +++ b/sdk/storage/azfile/assets.json @@ -2,5 +2,5 @@ "AssetsRepo": "Azure/azure-sdk-assets", "AssetsRepoPrefixPath": "go", "TagPrefix": "go/storage/azfile", - "Tag": "go/storage/azfile_f80b868396" + "Tag": "go/storage/azfile_8f8ed3dd66" } diff --git a/sdk/storage/azfile/internal/exported/shared_key_credential.go b/sdk/storage/azfile/internal/exported/shared_key_credential.go index 50aaf889942f..b2545b2bbb72 100644 --- a/sdk/storage/azfile/internal/exported/shared_key_credential.go +++ b/sdk/storage/azfile/internal/exported/shared_key_credential.go @@ -11,7 +11,9 @@ import ( "crypto/hmac" "crypto/sha256" "encoding/base64" + "errors" "fmt" + "github.com/Azure/azure-sdk-for-go/sdk/internal/errorinfo" "net/http" "net/url" "sort" @@ -195,6 +197,17 @@ func NewSharedKeyCredPolicy(cred *SharedKeyCredential) *SharedKeyCredPolicy { } func (s *SharedKeyCredPolicy) Do(req *policy.Request) (*http.Response, error) { + // skip adding the authorization header if no SharedKeyCredential was provided. + // this prevents a panic that might be hard to diagnose and allows testing + // against http endpoints that don't require authentication. + if s.cred == nil { + return req.Next() + } + + if err := checkHTTPSForAuth(req); err != nil { + return nil, err + } + if d := getHeader(shared.HeaderXmsDate, req.Raw().Header); d == "" { req.Raw().Header.Set(shared.HeaderXmsDate, time.Now().UTC().Format(http.TimeFormat)) } @@ -216,3 +229,10 @@ func (s *SharedKeyCredPolicy) Do(req *policy.Request) (*http.Response, error) { } return response, err } + +func checkHTTPSForAuth(req *policy.Request) error { + if strings.ToLower(req.Raw().URL.Scheme) != "https" { + return errorinfo.NonRetriableError(errors.New("authenticated requests are not permitted for non TLS protected (https) endpoints")) + } + return nil +} diff --git a/sdk/storage/azfile/service/client_test.go b/sdk/storage/azfile/service/client_test.go index 0d83e8556f30..61061fcbb121 100644 --- a/sdk/storage/azfile/service/client_test.go +++ b/sdk/storage/azfile/service/client_test.go @@ -630,6 +630,34 @@ func (s *ServiceRecordedTestsSuite) TestPremiumAccountListShares() { } } +func (s *ServiceRecordedTestsSuite) TestServiceClientRejectHTTP() { + _require := require.New(s.T()) + + cred, err := testcommon.GetGenericSharedKeyCredential(testcommon.TestAccountDefault) + _require.NoError(err) + + svcClient, err := service.NewClientWithSharedKeyCredential("http://"+cred.AccountName()+".file.core.windows.net/", cred, nil) + _require.NoError(err) + + _, err = svcClient.GetProperties(context.Background(), nil) + _require.Error(err) +} + +func (s *ServiceRecordedTestsSuite) TestServiceClientWithNilSharedKey() { + _require := require.New(s.T()) + + accountName, _ := testcommon.GetGenericAccountInfo(testcommon.TestAccountDefault) + _require.Greater(len(accountName), 0) + + options := &service.ClientOptions{} + testcommon.SetClientOptions(s.T(), &options.ClientOptions) + svcClient, err := service.NewClientWithSharedKeyCredential("https://"+accountName+".file.core.windows.net/", nil, options) + _require.NoError(err) + + _, err = svcClient.GetProperties(context.Background(), nil) + _require.Error(err) +} + func (s *ServiceRecordedTestsSuite) TestServiceClientCustomAudience() { _require := require.New(s.T()) testName := s.T().Name()