diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index f15d2cd19..9076dc54f 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -271,42 +271,36 @@ func (iam *IdentityAccessManagement) loadS3ApiConfiguration(config *iam_pb.S3Api for _, account := range config.Accounts { glog.V(3).Infof("loading account name=%s, id=%s", account.DisplayName, account.Id) + accounts[account.Id] = &Account{ + Id: account.Id, + DisplayName: account.DisplayName, + EmailAddress: account.EmailAddress, + } switch account.Id { case AccountAdmin.Id: - AccountAdmin = Account{ - Id: account.Id, - DisplayName: account.DisplayName, - EmailAddress: account.EmailAddress, - } - accounts[account.Id] = &AccountAdmin foundAccountAdmin = true case AccountAnonymous.Id: - AccountAnonymous = Account{ - Id: account.Id, - DisplayName: account.DisplayName, - EmailAddress: account.EmailAddress, - } - accounts[account.Id] = &AccountAnonymous foundAccountAnonymous = true - default: - t := Account{ - Id: account.Id, - DisplayName: account.DisplayName, - EmailAddress: account.EmailAddress, - } - accounts[account.Id] = &t } if account.EmailAddress != "" { emailAccount[account.EmailAddress] = accounts[account.Id] } } if !foundAccountAdmin { - accounts[AccountAdmin.Id] = &AccountAdmin - emailAccount[AccountAdmin.EmailAddress] = &AccountAdmin + accounts[AccountAdmin.Id] = &Account{ + DisplayName: AccountAdmin.DisplayName, + EmailAddress: AccountAdmin.EmailAddress, + Id: AccountAdmin.Id, + } + emailAccount[AccountAdmin.EmailAddress] = accounts[AccountAdmin.Id] } if !foundAccountAnonymous { - accounts[AccountAnonymous.Id] = &AccountAnonymous - emailAccount[AccountAnonymous.EmailAddress] = &AccountAnonymous + accounts[AccountAnonymous.Id] = &Account{ + DisplayName: AccountAnonymous.DisplayName, + EmailAddress: AccountAnonymous.EmailAddress, + Id: AccountAnonymous.Id, + } + emailAccount[AccountAnonymous.EmailAddress] = accounts[AccountAnonymous.Id] } for _, ident := range config.Identities { glog.V(3).Infof("loading identity %s (disabled=%v)", ident.Name, ident.Disabled) @@ -615,9 +609,8 @@ func (iam *IdentityAccessManagement) authRequestWithAuthType(r *http.Request, ac amzAuthType = "SigV4" case authTypeStreamingUnsigned: glog.V(3).Infof("unsigned streaming upload") - // no amzAuthType set for this case in original code? - // Actually original explicitly returned ErrNone without setting identity - return identity, s3err.ErrNone, reqAuthType + identity, s3Err = iam.reqSignatureV4Verify(r) + amzAuthType = "SigV4" case authTypeJWT: glog.V(3).Infof("jwt auth type detected, iamIntegration != nil? %t", iam.iamIntegration != nil) r.Header.Set(s3_constants.AmzAuthType, "Jwt") @@ -749,7 +742,8 @@ func (iam *IdentityAccessManagement) AuthSignatureOnly(r *http.Request) (*Identi case authTypeStreamingUnsigned: glog.V(3).Infof("unsigned streaming upload") - return identity, s3err.ErrNone + identity, s3Err = iam.reqSignatureV4Verify(r) + authType = "SigV4" case authTypeJWT: glog.V(3).Infof("jwt auth type detected, iamIntegration != nil? %t", iam.iamIntegration != nil) r.Header.Set(s3_constants.AmzAuthType, "Jwt") @@ -783,6 +777,9 @@ func (iam *IdentityAccessManagement) AuthSignatureOnly(r *http.Request) (*Identi } func (identity *Identity) canDo(action Action, bucket string, objectKey string) bool { + if identity == nil { + return false + } if identity.isAdmin() { return true } @@ -829,6 +826,9 @@ func (identity *Identity) canDo(action Action, bucket string, objectKey string) } func (identity *Identity) isAdmin() bool { + if identity == nil { + return false + } return slices.Contains(identity.Actions, s3_constants.ACTION_ADMIN) } diff --git a/weed/s3api/auth_security_test.go b/weed/s3api/auth_security_test.go new file mode 100644 index 000000000..553f6838c --- /dev/null +++ b/weed/s3api/auth_security_test.go @@ -0,0 +1,181 @@ +package s3api + +import ( + "context" + "crypto/sha256" + "fmt" + "net/http" + "net/http/httptest" + "os" + "testing" + "time" + + "github.com/aws/aws-sdk-go-v2/aws" + v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func signRawHTTPRequest(ctx context.Context, req *http.Request, accessKey, secretKey, region string) error { + creds := aws.Credentials{ + AccessKeyID: accessKey, + SecretAccessKey: secretKey, + } + signer := v4.NewSigner() + payloadHash := fmt.Sprintf("%x", sha256.Sum256([]byte{})) + return signer.SignHTTP(ctx, creds, req, payloadHash, "s3", region, time.Now()) +} + +func TestReproIssue7912(t *testing.T) { + // Create a temporary s3.json + configContent := `{ + "identities": [ + { + "name": "xx", + "credentials": [ + { + "accessKey": "xx_access_key", + "secretKey": "xx_secret_key" + } + ], + "actions": ["Admin", "Read", "Write", "List", "Tagging"] + }, + { + "name": "read_only_user", + "credentials": [ + { + "accessKey": "readonly_access_key", + "secretKey": "readonly_secret_key" + } + ], + "actions": ["Read", "List"] + } + ] +}` + tmpFile, err := os.CreateTemp("", "s3-config-*.json") + assert.NoError(t, err) + defer os.Remove(tmpFile.Name()) + + _, err = tmpFile.Write([]byte(configContent)) + assert.NoError(t, err) + tmpFile.Close() + + // Initialize Identities Access Management + option := &S3ApiServerOption{ + Config: tmpFile.Name(), + } + iam := NewIdentityAccessManagementWithStore(option, "memory") + + assert.True(t, iam.isEnabled(), "Auth should be enabled") + + // Test case 1: Unknown access key should be rejected + t.Run("Unknown access key", func(t *testing.T) { + r := httptest.NewRequest(http.MethodGet, "http://localhost:8333/", nil) + r.Host = "localhost:8333" + err := signRawHTTPRequest(context.Background(), r, "unknown_key", "any_secret", "us-east-1") + require.NoError(t, err) + + identity, errCode := iam.authRequest(r, s3_constants.ACTION_LIST) + assert.Equal(t, s3err.ErrInvalidAccessKeyID, errCode, "Should be denied with unknown access key") + assert.Nil(t, identity) + }) + + t.Run("Positive test case: properly signed credentials", func(t *testing.T) { + r := httptest.NewRequest(http.MethodGet, "http://localhost:8333/", nil) + r.Host = "localhost:8333" + err := signRawHTTPRequest(context.Background(), r, "readonly_access_key", "readonly_secret_key", "us-east-1") + require.NoError(t, err) + + identity, errCode := iam.authRequest(r, s3_constants.ACTION_LIST) + assert.Equal(t, s3err.ErrNone, errCode) + require.NotNil(t, identity) + assert.Equal(t, "read_only_user", identity.Name) + }) + + t.Run("Nil identity tests for guards", func(t *testing.T) { + var nilIdentity *Identity + // Test isAdmin guard + assert.False(t, nilIdentity.isAdmin()) + // Test canDo guard + assert.False(t, nilIdentity.canDo(s3_constants.ACTION_LIST, "bucket", "object")) + }) + + t.Run("AuthSignatureOnly path", func(t *testing.T) { + // Valid request + r := httptest.NewRequest(http.MethodGet, "http://localhost:8333/", nil) + r.Host = "localhost:8333" + err := signRawHTTPRequest(context.Background(), r, "xx_access_key", "xx_secret_key", "us-east-1") + require.NoError(t, err) + + identity, errCode := iam.AuthSignatureOnly(r) + assert.Equal(t, s3err.ErrNone, errCode) + require.NotNil(t, identity) + assert.Equal(t, "xx", identity.Name) + + // Invalid request (wrong signature) + r2 := httptest.NewRequest(http.MethodGet, "http://localhost:8333/", nil) + r2.Host = "localhost:8333" + err = signRawHTTPRequest(context.Background(), r2, "xx_access_key", "this_is_a_wrong_secret", "us-east-1") + require.NoError(t, err) + + _, errCode2 := iam.AuthSignatureOnly(r2) + assert.Equal(t, s3err.ErrSignatureDoesNotMatch, errCode2) + + // Verify fix: Streaming unsigned payload should be denied without auth header in AuthSignatureOnly + r3 := httptest.NewRequest(http.MethodPut, "http://localhost:8333/somebucket/someobject", nil) + r3.Header.Set("x-amz-content-sha256", "STREAMING-UNSIGNED-PAYLOAD-TRAILER") + // No Authorization header + _, errCode3 := iam.AuthSignatureOnly(r3) + assert.Equal(t, s3err.ErrAccessDenied, errCode3, "AuthSignatureOnly should be denied with unsigned streaming if no auth header") + }) + + t.Run("Wrong secret key", func(t *testing.T) { + r := httptest.NewRequest(http.MethodGet, "http://localhost:8333/", nil) + r.Host = "localhost:8333" + err := signRawHTTPRequest(context.Background(), r, "readonly_access_key", "this_is_a_wrong_secret", "us-east-1") + require.NoError(t, err) + + identity, errCode := iam.authRequest(r, s3_constants.ACTION_LIST) + assert.Equal(t, s3err.ErrSignatureDoesNotMatch, errCode, "Should NOT be allowed with wrong signature") + assert.Nil(t, identity) + }) + + t.Run("Anonymous request to protected bucket", func(t *testing.T) { + r := httptest.NewRequest(http.MethodGet, "http://localhost:8333/somebucket/", nil) + // No Authorization header + + identity, errCode := iam.authRequest(r, s3_constants.ACTION_LIST) + assert.Equal(t, s3err.ErrAccessDenied, errCode, "Should be denied for anonymous") + assert.Nil(t, identity) + }) + + t.Run("Non-S3 request should be denied", func(t *testing.T) { + r := httptest.NewRequest(http.MethodGet, "http://localhost:8333/", nil) + // No headers at all + + identity, errCode := iam.authRequest(r, s3_constants.ACTION_LIST) + assert.Equal(t, s3err.ErrAccessDenied, errCode) + assert.Nil(t, identity) + }) + t.Run("Any other credentials", func(t *testing.T) { + r := httptest.NewRequest(http.MethodGet, "http://localhost:8333/", nil) + r.Host = "localhost:8333" + err := signRawHTTPRequest(context.Background(), r, "some_other_key", "some_secret", "us-east-1") + require.NoError(t, err) + + identity, errCode := iam.authRequest(r, s3_constants.ACTION_LIST) + assert.Equal(t, s3err.ErrInvalidAccessKeyID, errCode, "Should NOT be allowed with ANY other credentials") + assert.Nil(t, identity) + }) + t.Run("Streaming unsigned payload bypass attempt", func(t *testing.T) { + r := httptest.NewRequest(http.MethodPut, "http://localhost:8333/somebucket/someobject", nil) + r.Header.Set("x-amz-content-sha256", "STREAMING-UNSIGNED-PAYLOAD-TRAILER") + // No Authorization header + + identity, errCode := iam.authRequest(r, s3_constants.ACTION_WRITE) + assert.Equal(t, s3err.ErrAccessDenied, errCode, "Should be denied with unsigned streaming if no auth header") + assert.Nil(t, identity) + }) +}