Skip to content

[#681] acl: Prepare owner ID before processing #8

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 24 additions & 21 deletions api/handler/acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,10 @@ const (
aclRead AWSACL = "READ"
)

// GranteeType is aws grantee permission type constants.
type GranteeType string

const (
acpCanonicalUser GranteeType = "CanonicalUser"
acpAmazonCustomerByEmail GranteeType = "AmazonCustomerByEmail"
acpGroup GranteeType = "Group"
acpCanonicalUser = "CanonicalUser"
acpAmazonCustomerByEmail = "AmazonCustomerByEmail"
acpGroup = "Group"
)

type bucketPolicy struct {
Expand Down Expand Up @@ -220,6 +217,9 @@ func (h *handler) PutBucketACLHandler(w http.ResponseWriter, r *http.Request) {
} else if err = xml.NewDecoder(r.Body).Decode(list); err != nil {
h.logAndSendError(w, "could not parse bucket acl", reqInfo, errors.GetAPIError(errors.ErrMalformedXML))
return
} else {
// workaround to decode owner ID
list.Owner.ID = hex.EncodeToString(key.Bytes())
}

resInfo := &resourceInfo{Bucket: reqInfo.BucketName}
Expand Down Expand Up @@ -355,6 +355,9 @@ func (h *handler) PutObjectACLHandler(w http.ResponseWriter, r *http.Request) {
} else if err = xml.NewDecoder(r.Body).Decode(list); err != nil {
h.logAndSendError(w, "could not parse bucket acl", reqInfo, errors.GetAPIError(errors.ErrMalformedXML))
return
} else {
// workaround to decode owner ID
list.Owner.ID = hex.EncodeToString(key.Bytes())
}

resInfo := &resourceInfo{
Expand Down Expand Up @@ -469,7 +472,7 @@ func parseACLHeaders(header http.Header, key *keys.PublicKey) (*AccessControlPol
Grantee: &Grantee{
ID: hex.EncodeToString(key.Bytes()),
DisplayName: key.Address(),
Type: acpCanonicalUser,
Type: formGranteeType(acpCanonicalUser),
},
Permission: aclFullControl,
}}
Expand Down Expand Up @@ -509,7 +512,7 @@ func addGrantees(list []*Grant, headers http.Header, hdr string) ([]*Grant, erro
}

for _, grantee := range grantees {
if grantee.Type == acpAmazonCustomerByEmail || (grantee.Type == acpGroup && grantee.URI != allUsersGroup) {
if grantee.matchType(acpAmazonCustomerByEmail) || (grantee.matchType(acpGroup) && grantee.URI != allUsersGroup) {
return nil, stderrors.New("unsupported grantee type")
}

Expand Down Expand Up @@ -559,17 +562,17 @@ func formGrantee(granteeType, value string) (*Grantee, error) {
case "id":
return &Grantee{
ID: value,
Type: acpCanonicalUser,
Type: formGranteeType(acpCanonicalUser),
}, nil
case "uri":
return &Grantee{
URI: value,
Type: acpGroup,
Type: formGranteeType(acpGroup),
}, nil
case "emailAddress":
return &Grantee{
EmailAddress: value,
Type: acpAmazonCustomerByEmail,
Type: formGranteeType(acpAmazonCustomerByEmail),
}, nil
}
// do not return grantee type to avoid sensitive data logging (#489)
Expand All @@ -583,7 +586,7 @@ func addPredefinedACP(acp *AccessControlPolicy, cannedACL string) (*AccessContro
acp.AccessControlList = append(acp.AccessControlList, &Grant{
Grantee: &Grantee{
URI: allUsersGroup,
Type: acpGroup,
Type: formGranteeType(acpGroup),
},
Permission: aclFullControl,
})
Expand All @@ -593,7 +596,7 @@ func addPredefinedACP(acp *AccessControlPolicy, cannedACL string) (*AccessContro
acp.AccessControlList = append(acp.AccessControlList, &Grant{
Grantee: &Grantee{
URI: allUsersGroup,
Type: acpGroup,
Type: formGranteeType(acpGroup),
},
Permission: aclRead,
})
Expand Down Expand Up @@ -1173,12 +1176,12 @@ func aclToAst(acl *AccessControlPolicy, resInfo *resourceInfo) (*ast, error) {
}

for _, grant := range acl.AccessControlList {
if grant.Grantee.Type == acpAmazonCustomerByEmail || (grant.Grantee.Type == acpGroup && grant.Grantee.URI != allUsersGroup) {
if grant.Grantee.matchType(acpAmazonCustomerByEmail) || (grant.Grantee.matchType(acpGroup) && grant.Grantee.URI != allUsersGroup) {
return nil, stderrors.New("unsupported grantee type")
}

var groupGrantee bool
if grant.Grantee.Type == acpGroup {
if grant.Grantee.matchType(acpGroup) {
groupGrantee = true
} else if grant.Grantee.ID == acl.Owner.ID {
continue
Expand Down Expand Up @@ -1212,12 +1215,12 @@ func aclToPolicy(acl *AccessControlPolicy, resInfo *resourceInfo) (*bucketPolicy
}

for _, grant := range acl.AccessControlList {
if grant.Grantee.Type == acpAmazonCustomerByEmail || (grant.Grantee.Type == acpGroup && grant.Grantee.URI != allUsersGroup) {
if grant.Grantee.matchType(acpAmazonCustomerByEmail) || (grant.Grantee.matchType(acpGroup) && grant.Grantee.URI != allUsersGroup) {
return nil, stderrors.New("unsupported grantee type")
}

user := grant.Grantee.ID
if grant.Grantee.Type == acpGroup {
if grant.Grantee.matchType(acpGroup) {
user = allUsersWildcard
} else if user == acl.Owner.ID {
continue
Expand Down Expand Up @@ -1377,10 +1380,10 @@ func (h *handler) encodeObjectACL(bucketACL *layer.BucketACL, bucketName, object

var grantee *Grantee
if key == allUsersGroup {
grantee = NewGrantee(acpGroup)
grantee = NewGrantee(formGranteeType(acpGroup))
grantee.URI = allUsersGroup
} else {
grantee = NewGrantee(acpCanonicalUser)
grantee = NewGrantee(formGranteeType(acpCanonicalUser))
grantee.ID = key
}

Expand Down Expand Up @@ -1453,7 +1456,7 @@ func bucketACLToTable(acp *AccessControlPolicy, resInfo *resourceInfo) (*eacl.Ta
}

func getRecordFunction(grantee *Grantee) (getRecordFunc, error) {
switch grantee.Type {
switch grantee.TypeString() {
case acpAmazonCustomerByEmail:
case acpCanonicalUser:
pk, err := keys.NewPublicKeyFromString(grantee.ID)
Expand All @@ -1473,7 +1476,7 @@ func getRecordFunction(grantee *Grantee) (getRecordFunc, error) {

func isValidGrant(grant *Grant) bool {
return (grant.Permission == aclFullControl || grant.Permission == aclRead || grant.Permission == aclWrite) &&
(grant.Grantee.Type == acpCanonicalUser || (grant.Grantee.Type == acpGroup && grant.Grantee.URI == allUsersGroup))
(grant.Grantee.matchType(acpCanonicalUser) || (grant.Grantee.matchType(acpGroup) && grant.Grantee.URI == allUsersGroup))
}

func getAllowRecord(op eacl.Operation, pk *keys.PublicKey) *eacl.Record {
Expand Down
85 changes: 65 additions & 20 deletions api/handler/acl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"crypto/sha256"
"encoding/hex"
"encoding/json"
"encoding/xml"
"fmt"
"io"
"net/http"
Expand Down Expand Up @@ -724,13 +725,13 @@ func TestBucketAclToPolicy(t *testing.T) {
AccessControlList: []*Grant{{
Grantee: &Grantee{
URI: allUsersGroup,
Type: acpGroup,
Type: formGranteeType(acpGroup),
},
Permission: aclRead,
}, {
Grantee: &Grantee{
ID: id2,
Type: acpCanonicalUser,
Type: formGranteeType(acpCanonicalUser),
},
Permission: aclWrite,
}},
Expand Down Expand Up @@ -790,19 +791,19 @@ func TestObjectAclToPolicy(t *testing.T) {
AccessControlList: []*Grant{{
Grantee: &Grantee{
ID: id,
Type: acpCanonicalUser,
Type: formGranteeType(acpCanonicalUser),
},
Permission: aclFullControl,
}, {
Grantee: &Grantee{
ID: id2,
Type: acpCanonicalUser,
Type: formGranteeType(acpCanonicalUser),
},
Permission: aclFullControl,
}, {
Grantee: &Grantee{
URI: allUsersGroup,
Type: acpGroup,
Type: formGranteeType(acpGroup),
},
Permission: aclRead,
}},
Expand Down Expand Up @@ -859,7 +860,7 @@ func TestObjectWithVersionAclToTable(t *testing.T) {
AccessControlList: []*Grant{{
Grantee: &Grantee{
ID: id,
Type: acpCanonicalUser,
Type: formGranteeType(acpCanonicalUser),
},
Permission: aclFullControl,
}},
Expand Down Expand Up @@ -980,13 +981,13 @@ func TestParseCannedACLHeaders(t *testing.T) {
Grantee: &Grantee{
ID: id,
DisplayName: address,
Type: acpCanonicalUser,
Type: formGranteeType(acpCanonicalUser),
},
Permission: aclFullControl,
}, {
Grantee: &Grantee{
URI: allUsersGroup,
Type: acpGroup,
Type: formGranteeType(acpGroup),
},
Permission: aclRead,
}},
Expand Down Expand Up @@ -1021,37 +1022,37 @@ func TestParseACLHeaders(t *testing.T) {
Grantee: &Grantee{
ID: id,
DisplayName: address,
Type: acpCanonicalUser,
Type: formGranteeType(acpCanonicalUser),
},
Permission: aclFullControl,
}, {
Grantee: &Grantee{
ID: "user1",
Type: acpCanonicalUser,
Type: formGranteeType(acpCanonicalUser),
},
Permission: aclFullControl,
}, {
Grantee: &Grantee{
URI: allUsersGroup,
Type: acpGroup,
Type: formGranteeType(acpGroup),
},
Permission: aclRead,
}, {
Grantee: &Grantee{
ID: "user2",
Type: acpCanonicalUser,
Type: formGranteeType(acpCanonicalUser),
},
Permission: aclRead,
}, {
Grantee: &Grantee{
ID: "user2",
Type: acpCanonicalUser,
Type: formGranteeType(acpCanonicalUser),
},
Permission: aclWrite,
}, {
Grantee: &Grantee{
ID: "user3",
Type: acpCanonicalUser,
Type: formGranteeType(acpCanonicalUser),
},
Permission: aclWrite,
}},
Expand Down Expand Up @@ -1112,13 +1113,13 @@ func TestBucketAclToTable(t *testing.T) {
AccessControlList: []*Grant{{
Grantee: &Grantee{
URI: allUsersGroup,
Type: acpGroup,
Type: formGranteeType(acpGroup),
},
Permission: aclRead,
}, {
Grantee: &Grantee{
ID: id2,
Type: acpCanonicalUser,
Type: formGranteeType(acpCanonicalUser),
},
Permission: aclWrite,
}},
Expand Down Expand Up @@ -1169,13 +1170,13 @@ func TestObjectAclToAst(t *testing.T) {
AccessControlList: []*Grant{{
Grantee: &Grantee{
ID: id,
Type: acpCanonicalUser,
Type: formGranteeType(acpCanonicalUser),
},
Permission: aclFullControl,
}, {
Grantee: &Grantee{
ID: id2,
Type: acpCanonicalUser,
Type: formGranteeType(acpCanonicalUser),
},
Permission: aclRead,
},
Expand Down Expand Up @@ -1238,13 +1239,13 @@ func TestBucketAclToAst(t *testing.T) {
{
Grantee: &Grantee{
ID: id2,
Type: acpCanonicalUser,
Type: formGranteeType(acpCanonicalUser),
},
Permission: aclWrite,
}, {
Grantee: &Grantee{
URI: allUsersGroup,
Type: acpGroup,
Type: formGranteeType(acpGroup),
},
Permission: aclRead,
},
Expand Down Expand Up @@ -1436,3 +1437,47 @@ func putBucketACL(t *testing.T, tc *handlerContext, bktName string, box *accessb
tc.Handler().PutBucketACLHandler(w, r)
assertStatus(t, w, http.StatusOK)
}

func TestACLGranteeParse(t *testing.T) {
body := []byte(`
<AccessControlPolicy xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
<Owner>
<DisplayName>NbUgTSFvPmsRxmGeWpuuGeJUoRoi6PErcM</DisplayName>
<ID>NbUgTSFvPmsRxmGeWpuuGeJUoRoi6PErcM</ID>
</Owner>
<AccessControlList>
<Grant>
<Grantee xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="CanonicalUser">
<ID>031a6c6fbbdf02ca351745fa86b9ba5a9452d785ac4f7fc2b7548ca2a46c4fcf4a</ID>
</Grantee>
<Permission>FULL_CONTROL</Permission>
</Grant>
<Grant>
<Grantee xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="Group">
<URI>http://acs.amazonaws.com/groups/global/AllUsers</URI>
</Grantee>
<Permission>FULL_CONTROL</Permission>
</Grant>
</AccessControlList>
</AccessControlPolicy>
`)

acl := &AccessControlPolicy{}
err := xml.Unmarshal(body, acl)
require.NoError(t, err)
require.True(t, acl.AccessControlList[0].Grantee.matchType(acpCanonicalUser))
require.True(t, acl.AccessControlList[1].Grantee.matchType(acpGroup))

grantee := NewGrantee(formGranteeType(acpGroup))
grantee.URI = allUsersGroup

raw, err := xml.MarshalIndent(grantee, "", " ")
require.NoError(t, err)

grantee2 := &Grantee{}
err = xml.Unmarshal(raw, grantee2)
require.NoError(t, err)

grantee2.XMLNS.Value = granteeXMLNS
require.Equal(t, grantee, grantee2)
}
Loading