S3: Signature verification should not check permissions (#7335)

* Signature verification should not check permissions - that's done later in authRequest

* test permissions during signature verfication

* fix s3 test path

* s3tests_boto3 => s3tests

* remove extra lines
This commit is contained in:
Chris Lu
2025-10-15 11:27:39 -07:00
committed by GitHub
parent ffc45a538d
commit 3d25f206c8
5 changed files with 420 additions and 386 deletions

View File

@@ -3,6 +3,7 @@ package s3api
import (
"os"
"reflect"
"sync"
"testing"
"github.com/seaweedfs/seaweedfs/weed/credential"
@@ -543,3 +544,58 @@ func TestListBucketsAuthRequest(t *testing.T) {
t.Log("ListBuckets operation bypasses global permission check when bucket is empty")
t.Log("Object listing still properly enforces bucket-level permissions")
}
// TestSignatureVerificationDoesNotCheckPermissions tests that signature verification
// only validates the signature and identity, not permissions. Permissions should be
// checked later in authRequest based on the actual operation.
// This test validates the fix for issue #7334
func TestSignatureVerificationDoesNotCheckPermissions(t *testing.T) {
t.Run("List-only user can authenticate via signature", func(t *testing.T) {
// Create IAM with a user that only has List permissions on specific buckets
iam := &IdentityAccessManagement{
hashes: make(map[string]*sync.Pool),
hashCounters: make(map[string]*int32),
}
err := iam.loadS3ApiConfiguration(&iam_pb.S3ApiConfiguration{
Identities: []*iam_pb.Identity{
{
Name: "list-only-user",
Credentials: []*iam_pb.Credential{
{
AccessKey: "list_access_key",
SecretKey: "list_secret_key",
},
},
Actions: []string{
"List:bucket-123",
"Read:bucket-123",
},
},
},
})
assert.NoError(t, err)
// Before the fix, signature verification would fail because it checked for Write permission
// After the fix, signature verification should succeed (only checking signature validity)
// The actual permission check happens later in authRequest with the correct action
// The user should be able to authenticate (signature verification passes)
// But authorization for specific actions is checked separately
identity, cred, found := iam.lookupByAccessKey("list_access_key")
assert.True(t, found, "Should find the user by access key")
assert.Equal(t, "list-only-user", identity.Name)
assert.Equal(t, "list_secret_key", cred.SecretKey)
// User should have the correct permissions
assert.True(t, identity.canDo(Action(ACTION_LIST), "bucket-123", ""))
assert.True(t, identity.canDo(Action(ACTION_READ), "bucket-123", ""))
// User should NOT have write permissions
assert.False(t, identity.canDo(Action(ACTION_WRITE), "bucket-123", ""))
})
t.Log("This test validates the fix for issue #7334")
t.Log("Signature verification no longer checks for Write permission")
t.Log("This allows list-only and read-only users to authenticate via AWS Signature V4")
}

View File

@@ -116,11 +116,6 @@ func (iam *IdentityAccessManagement) doesSignV2Match(r *http.Request) (*Identity
return nil, s3err.ErrInvalidAccessKeyID
}
bucket, object := s3_constants.GetBucketAndObject(r)
if !identity.canDo(s3_constants.ACTION_WRITE, bucket, object) {
return nil, s3err.ErrAccessDenied
}
expectedAuth := signatureV2(cred, r.Method, r.URL.Path, r.URL.Query().Encode(), r.Header)
if !compareSignatureV2(v2Auth, expectedAuth) {
return nil, s3err.ErrSignatureDoesNotMatch
@@ -163,11 +158,6 @@ func (iam *IdentityAccessManagement) doesPresignV2SignatureMatch(r *http.Request
return nil, s3err.ErrInvalidAccessKeyID
}
bucket, object := s3_constants.GetBucketAndObject(r)
if !identity.canDo(s3_constants.ACTION_READ, bucket, object) {
return nil, s3err.ErrAccessDenied
}
expectedSignature := preSignatureV2(cred, r.Method, r.URL.Path, r.URL.Query().Encode(), r.Header, expires)
if !compareSignatureV2(signature, expectedSignature) {
return nil, s3err.ErrSignatureDoesNotMatch

View File

@@ -190,12 +190,6 @@ func (iam *IdentityAccessManagement) doesSignatureMatch(hashedPayload string, r
return nil, s3err.ErrInvalidAccessKeyID
}
bucket, object := s3_constants.GetBucketAndObject(r)
canDoResult := identity.canDo(s3_constants.ACTION_WRITE, bucket, object)
if !canDoResult {
return nil, s3err.ErrAccessDenied
}
// Extract date, if not present throw error.
var dateStr string
if dateStr = req.Header.Get("x-amz-date"); dateStr == "" {
@@ -331,12 +325,6 @@ func (iam *IdentityAccessManagement) doesPresignedSignatureMatch(hashedPayload s
return nil, s3err.ErrInvalidAccessKeyID
}
// Check permissions
bucket, object := s3_constants.GetBucketAndObject(r)
if !identity.canDo(s3_constants.ACTION_READ, bucket, object) {
return nil, s3err.ErrAccessDenied
}
// Parse date
t, e := time.Parse(iso8601Format, dateStr)
if e != nil {