Skip to content

Commit

Permalink
Use PutBucketOwnershipControls instead of PutBucketPolicy (#1026)
Browse files Browse the repository at this point in the history 
Closes #1023.
  • Loading branch information
@roman-khimov
roman-khimov authored Nov 21, 2024
2 parents 231599c + 2014650 commit 811d52f
Show file tree
Hide file tree
Showing 12 changed files with 557 additions and 131 deletions.
219 changes: 170 additions & 49 deletions api/handler/acl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (
"strconv"
"strings"

"github.com/nspcc-dev/neo-go/pkg/crypto/hash"
"github.com/nspcc-dev/neo-go/pkg/encoding/address"
"github.com/nspcc-dev/neofs-s3-gw/api"
"github.com/nspcc-dev/neofs-s3-gw/api/data"
"github.com/nspcc-dev/neofs-s3-gw/api/layer"
Expand All @@ -25,6 +27,31 @@ import (
"go.uber.org/zap"
)

var (
// There are special user.IDs for ACL marker ownership records.

// NKuyBkoGdZZSLyPbJEetheRhMjezqzTxcW.
ownerEnforcedUserID user.ID
// NKuyBkoGdZZSLyPbJEetheRhMjezwhg9vh.
ownerPreferredUserID user.ID
// NKuyBkoGdZZSLyPbJEetheRhMjf17rTWMC.
ownerObjectWriterUserID user.ID
)

func init() {
ownerEnforcedUserID[0] = address.Prefix
ownerEnforcedUserID[20] = 1
copy(ownerEnforcedUserID[21:], hash.Checksum(ownerEnforcedUserID[:21]))

ownerPreferredUserID[0] = address.Prefix
ownerPreferredUserID[20] = 2
copy(ownerPreferredUserID[21:], hash.Checksum(ownerPreferredUserID[:21]))

ownerObjectWriterUserID[0] = address.Prefix
ownerObjectWriterUserID[20] = 3
copy(ownerObjectWriterUserID[21:], hash.Checksum(ownerObjectWriterUserID[:21]))
}

var (
writeOps = []eacl.Operation{eacl.OperationPut, eacl.OperationDelete}
writeOpsMap = map[eacl.Operation]struct{}{
Expand Down Expand Up @@ -393,6 +420,14 @@ func (h *handler) PutObjectACLHandler(w http.ResponseWriter, r *http.Request) {
r.Header.Set(api.AmzACL, "")
}

if isBucketOwnerPreferred(eacl.EACL) {
if !isValidOwnerPreferred(r) {
h.logAndSendError(w, "header x-amz-acl:bucket-owner-full-control must be set", reqInfo, s3errors.GetAPIError(s3errors.ErrAccessDenied))
return
}
r.Header.Set(api.AmzACL, "")
}

p := &layer.HeadObjectParams{
BktInfo: bktInfo,
Object: reqInfo.ObjectName,
Expand Down Expand Up @@ -965,18 +1000,27 @@ func tryServiceRecord(record eacl.Record) *ServiceRecord {
func formRecords(resource *astResource) ([]*eacl.Record, error) {
var res []*eacl.Record

if resource.Version == amzBucketOwnerEnforced && resource.Object == amzBucketOwnerEnforced {
res = append(res, bucketOwnerEnforcedRecord())
return res, nil
}

if resource.Version == aclEnabledObjectWriter && resource.Object == aclEnabledObjectWriter {
res = append(res, bucketACLObjectWriterRecord())
return res, nil
}

for i := len(resource.Operations) - 1; i >= 0; i-- {
astOp := resource.Operations[i]

if len(astOp.Users) == 1 {
var markerRecord *eacl.Record

switch astOp.Users[0] {
case ownerEnforcedUserID:
markerRecord = bucketOwnerEnforcedRecord()
case ownerPreferredUserID:
markerRecord = bucketOwnerPreferredRecord()
case ownerObjectWriterUserID:
markerRecord = bucketACLObjectWriterRecord()
}

if markerRecord != nil {
res = append(res, markerRecord)
return res, nil
}
}

record := eacl.NewRecord()
record.SetOperation(astOp.Op)
record.SetAction(astOp.Action)
Expand Down Expand Up @@ -1014,8 +1058,9 @@ func addToList(operations []*astOperation, rec eacl.Record, target eacl.Target)
)

for _, astOp := range operations {
if astOp.Op == rec.Operation() && astOp.IsGroupGrantee() == groupTarget {
if astOp.Op == rec.Operation() && astOp.Action == rec.Action() && astOp.IsGroupGrantee() == groupTarget {
found = astOp
break
}
}

Expand Down Expand Up @@ -1057,40 +1102,6 @@ func policyToAst(bktPolicy *bucketPolicy) (*ast, error) {
rr := make(map[string]*astResource)

for _, state := range bktPolicy.Statement {
if state.Sid == "BucketOwnerEnforced" &&
state.Action.Equal(stringOrSlice{values: []string{"*"}}) &&
state.Effect == "Deny" &&
state.Resource.Equal(stringOrSlice{values: []string{"*"}}) {
if conditionObj, ok := state.Condition["StringNotEquals"]; ok {
if val := conditionObj["s3:x-amz-object-ownership"]; val == amzBucketOwnerEnforced {
rr[amzBucketOwnerEnforced] = &astResource{
resourceInfo: resourceInfo{
Version: amzBucketOwnerEnforced,
Object: amzBucketOwnerEnforced,
},
}

continue
}
}

return nil, fmt.Errorf("unsupported ownership: %v", state.Principal)
}

if state.Sid == "BucketEnableACL" &&
state.Action.Equal(stringOrSlice{values: []string{"s3:PutObject"}}) &&
state.Effect == "Allow" &&
state.Resource.Equal(stringOrSlice{values: []string{"*"}}) {
rr[aclEnabledObjectWriter] = &astResource{
resourceInfo: resourceInfo{
Version: aclEnabledObjectWriter,
Object: aclEnabledObjectWriter,
},
}

continue
}

if state.Principal.AWS != "" && state.Principal.AWS != allUsersWildcard ||
state.Principal.AWS == "" && state.Principal.CanonicalUser == "" {
return nil, fmt.Errorf("unsupported principal: %v", state.Principal)
Expand Down Expand Up @@ -1666,6 +1677,10 @@ func bucketOwnerEnforcedRecord() *eacl.Record {
amzBucketOwnerEnforced,
)

t := eacl.NewTarget()
t.SetAccounts([]user.ID{ownerEnforcedUserID})
markerRecord.SetTargets(*t)

return markerRecord
}

Expand All @@ -1685,14 +1700,18 @@ func isValidOwnerEnforced(r *http.Request) bool {
}

func bucketACLObjectWriterRecord() *eacl.Record {
var markerRecord = eacl.CreateRecord(eacl.ActionAllow, eacl.OperationPut)
var markerRecord = eacl.CreateRecord(eacl.ActionDeny, eacl.OperationPut)
markerRecord.AddFilter(
eacl.HeaderFromRequest,
eacl.MatchStringEqual,
eacl.MatchStringNotEqual,
amzBucketOwnerField,
aclEnabledObjectWriter,
amzBucketOwnerObjectWriter,
)

t := eacl.NewTarget()
t.SetAccounts([]user.ID{ownerObjectWriterUserID})
markerRecord.SetTargets(*t)

return markerRecord
}

Expand All @@ -1708,7 +1727,109 @@ func isBucketOwnerForced(table *eacl.Table) bool {
f.Value() == amzBucketOwnerEnforced &&
f.From() == eacl.HeaderFromRequest &&
f.Matcher() == eacl.MatchStringNotEqual {
return true
if len(r.Targets()) == 1 && len(r.Targets()[0].Accounts()) == 1 {
return r.Targets()[0].Accounts()[0] == ownerEnforcedUserID
}
}
}
}
}

return false
}

func bucketOwnerPreferredRecord() *eacl.Record {
var markerRecord = eacl.CreateRecord(eacl.ActionDeny, eacl.OperationPut)
markerRecord.AddFilter(
eacl.HeaderFromRequest,
eacl.MatchStringNotEqual,
amzBucketOwnerField,
amzBucketOwnerPreferred,
)

t := eacl.NewTarget()
t.SetAccounts([]user.ID{ownerPreferredUserID})
markerRecord.SetTargets(*t)

return markerRecord
}

func isBucketOwnerPreferred(table *eacl.Table) bool {
if table == nil {
return false
}

for _, r := range table.Records() {
if r.Action() == eacl.ActionDeny && r.Operation() == eacl.OperationPut {
for _, f := range r.Filters() {
if f.Key() == amzBucketOwnerField &&
f.Value() == amzBucketOwnerPreferred &&
f.From() == eacl.HeaderFromRequest &&
f.Matcher() == eacl.MatchStringNotEqual {
if len(r.Targets()) == 1 && len(r.Targets()[0].Accounts()) == 1 {
return r.Targets()[0].Accounts()[0] == ownerPreferredUserID
}
}
}
}
}

return false
}

func isValidOwnerPreferred(r *http.Request) bool {
cannedACL := r.Header.Get(api.AmzACL)
return cannedACL == cannedACLBucketOwnerFullControl
}

func updateBucketOwnership(records []eacl.Record, newRecord *eacl.Record) []eacl.Record {
var (
rowID = -1
)

for index, r := range records {
if r.Action() == eacl.ActionDeny && r.Operation() == eacl.OperationPut {
for _, f := range r.Filters() {
if f.Key() == amzBucketOwnerField &&
f.From() == eacl.HeaderFromRequest &&
f.Matcher() == eacl.MatchStringNotEqual {
rowID = index
break
}
}
}

if rowID > -1 {
break
}
}

if rowID > -1 {
records = slices.Delete(records, rowID, rowID+1)
}

if newRecord != nil {
records = append(records, *newRecord)
}

return records
}

func isBucketOwnerObjectWriter(table *eacl.Table) bool {
if table == nil {
return false
}

for _, r := range table.Records() {
if r.Action() == eacl.ActionDeny && r.Operation() == eacl.OperationPut {
for _, f := range r.Filters() {
if f.Key() == amzBucketOwnerField &&
f.Value() == amzBucketOwnerObjectWriter &&
f.From() == eacl.HeaderFromRequest &&
f.Matcher() == eacl.MatchStringNotEqual {
if len(r.Targets()) == 1 && len(r.Targets()[0].Accounts()) == 1 {
return r.Targets()[0].Accounts()[0] == ownerObjectWriterUserID
}
}
}
}
Expand Down
56 changes: 42 additions & 14 deletions api/handler/acl_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import (
"crypto/sha256"
"encoding/hex"
"encoding/json"
"encoding/xml"
"fmt"
"io"
"net/http"
"net/http/httptest"
"testing"

"github.com/nspcc-dev/neo-go/pkg/crypto/keys"
Expand Down Expand Up @@ -1360,21 +1362,13 @@ func TestPutBucketACL(t *testing.T) {
// ACLs disabled.
putBucketACL(t, tc, bktName, box, header, http.StatusBadRequest)

aclPolicy := &bucketPolicy{
Statement: []statement{{
Sid: "BucketEnableACL",
Effect: "Allow",
Action: stringOrSlice{values: []string{"s3:PutObject"}},
Resource: stringOrSlice{values: []string{"*"}},
}},
}
putBucketPolicy(tc, bktName, aclPolicy, box, http.StatusOK)
putBucketOwnership(tc, bktName, box, amzBucketOwnerObjectWriter, http.StatusOK)

// ACLs enabled.
putBucketACL(t, tc, bktName, box, header, http.StatusOK)
header = map[string]string{api.AmzACL: "private"}
putBucketACL(t, tc, bktName, box, header, http.StatusOK)
checkLastRecords(t, tc, bktInfo, eacl.ActionDeny)
checkLastRecords(t, tc, bktInfo, eacl.ActionDeny, ownerObjectWriterUserID)
}

func TestBucketPolicy(t *testing.T) {
Expand All @@ -1390,7 +1384,13 @@ func TestBucketPolicy(t *testing.T) {
require.Equal(t, user.NewFromScriptHash(key.GetScriptHash()).String(), st.Principal.CanonicalUser)
require.Equal(t, []string{arnAwsPrefix + bktName}, st.Resource.values)
} else {
require.Equal(t, allUsersWildcard, st.Principal.AWS)
if st.Principal.AWS != "" {
require.Equal(t, allUsersWildcard, st.Principal.AWS)
} else {
// special marker record for ownership.
require.Equal(t, ownerEnforcedUserID.String(), st.Principal.CanonicalUser)
}

require.Equal(t, "Deny", st.Effect)
require.Equal(t, []string{arnAwsPrefix + bktName}, st.Resource.values)
}
Expand Down Expand Up @@ -1443,7 +1443,32 @@ func putBucketPolicy(hc *handlerContext, bktName string, bktPolicy *bucketPolicy
assertStatus(hc.t, w, status)
}

func checkLastRecords(t *testing.T, tc *handlerContext, bktInfo *data.BucketInfo, action eacl.Action) {
func putBucketOwnership(hc *handlerContext, bktName string, box *accessbox.Box, ownership string, status int) {
p := putBucketOwnershipControlsParams{
Rules: []objectOwnershipRules{
{
ObjectOwnership: ownership,
},
},
}

var b bytes.Buffer
err := xml.NewEncoder(&b).Encode(p)
require.NoError(hc.t, err)

w := httptest.NewRecorder()
r := httptest.NewRequest(http.MethodPut, defaultURL, &b)

reqInfo := api.NewReqInfo(w, r, api.ObjectRequest{Bucket: bktName})
r = r.WithContext(api.SetReqInfo(hc.Context(), reqInfo))
ctx := context.WithValue(r.Context(), api.BoxData, box)
r = r.WithContext(ctx)
hc.Handler().PutBucketOwnershipControlsHandler(w, r)

assertStatus(hc.t, w, status)
}

func checkLastRecords(t *testing.T, tc *handlerContext, bktInfo *data.BucketInfo, action eacl.Action, markerUserID user.ID) {
bktACL, err := tc.Layer().GetBucketACL(tc.Context(), bktInfo)
require.NoError(t, err)

Expand All @@ -1454,8 +1479,11 @@ func checkLastRecords(t *testing.T, tc *handlerContext, bktInfo *data.BucketInfo
}

for _, rec := range bktACL.EACL.Records()[length-7:] {
if rec.Action() != action || rec.Targets()[0].Role() != eacl.RoleOthers {
t.Fatalf("inavid last record: '%s', '%s', '%s',", rec.Action(), rec.Operation(), rec.Targets()[0].Role())
if rec.Targets()[0].Role() == eacl.RoleOthers {
require.Equal(t, action, rec.Action())
} else {
// special ownership marker rule.
require.Equal(t, markerUserID, rec.Targets()[0].Accounts()[0])
}
}
}
Expand Down
Loading

0 comments on commit 811d52f

Please sign in to comment.