From c98707607165de95fd303709acc5d47521491952 Mon Sep 17 00:00:00 2001 From: Denis Kirillov Date: Fri, 26 Aug 2022 17:47:55 +0300 Subject: [PATCH] [#681] acl: Prepare owner ID before processing Signed-off-by: Denis Kirillov --- api/handler/acl.go | 45 ++++++++++++---------- api/handler/acl_test.go | 85 +++++++++++++++++++++++++++++++---------- api/handler/response.go | 53 +++++++++++++++++++------ 3 files changed, 130 insertions(+), 53 deletions(-) diff --git a/api/handler/acl.go b/api/handler/acl.go index 0554568..809ce73 100644 --- a/api/handler/acl.go +++ b/api/handler/acl.go @@ -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 { @@ -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} @@ -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{ @@ -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, }} @@ -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") } @@ -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) @@ -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, }) @@ -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, }) @@ -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 @@ -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 @@ -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 } @@ -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) @@ -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 { diff --git a/api/handler/acl_test.go b/api/handler/acl_test.go index a47b169..6d131e2 100644 --- a/api/handler/acl_test.go +++ b/api/handler/acl_test.go @@ -8,6 +8,7 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" + "encoding/xml" "fmt" "io" "net/http" @@ -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, }}, @@ -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, }}, @@ -859,7 +860,7 @@ func TestObjectWithVersionAclToTable(t *testing.T) { AccessControlList: []*Grant{{ Grantee: &Grantee{ ID: id, - Type: acpCanonicalUser, + Type: formGranteeType(acpCanonicalUser), }, Permission: aclFullControl, }}, @@ -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, }}, @@ -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, }}, @@ -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, }}, @@ -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, }, @@ -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, }, @@ -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(` + + + NbUgTSFvPmsRxmGeWpuuGeJUoRoi6PErcM + NbUgTSFvPmsRxmGeWpuuGeJUoRoi6PErcM + + + + + 031a6c6fbbdf02ca351745fa86b9ba5a9452d785ac4f7fc2b7548ca2a46c4fcf4a + + FULL_CONTROL + + + + http://acs.amazonaws.com/groups/global/AllUsers + + FULL_CONTROL + + + +`) + + 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) +} diff --git a/api/handler/response.go b/api/handler/response.go index 70eb32d..859bef1 100644 --- a/api/handler/response.go +++ b/api/handler/response.go @@ -70,21 +70,50 @@ type Grant struct { // Grantee is info about access rights of some actor. type Grantee struct { - XMLName xml.Name `xml:"Grantee"` - XMLNS string `xml:"xmlns:xsi,attr"` - ID string `xml:"ID,omitempty"` - DisplayName string `xml:"DisplayName,omitempty"` - EmailAddress string `xml:"EmailAddress,omitempty"` - URI string `xml:"URI,omitempty"` - Type GranteeType `xml:"xsi:type,attr"` + XMLName xml.Name `xml:"Grantee"` + XMLNS xml.Attr `xml:"xsi,attr"` + ID string `xml:"ID,omitempty"` + DisplayName string `xml:"DisplayName,omitempty"` + EmailAddress string `xml:"EmailAddress,omitempty"` + URI string `xml:"URI,omitempty"` + Type xml.Attr `xml:"type,attr"` } -// NewGrantee creates new grantee using workaround -// https://github.com/golang/go/issues/9519#issuecomment-252196382 -func NewGrantee(t GranteeType) *Grantee { +func (g Grantee) matchType(t string) bool { + return g.Type.Value == t +} + +func (g Grantee) TypeString() string { + return g.Type.Value +} + +func formGranteeType(str string) xml.Attr { + return xml.Attr{ + Name: xml.Name{ + Space: "xsi", + Local: "type", + }, + Value: str, + } +} + +const granteeXMLNS = "http://www.w3.org/2001/XMLSchema-instance" + +// NewGrantee creates new grantee. +func NewGrantee(t xml.Attr) *Grantee { return &Grantee{ - XMLNS: "http://www.w3.org/2001/XMLSchema-instance", - Type: t, + XMLName: xml.Name{ + Local: "Grantee", + }, + + XMLNS: xml.Attr{ + Name: xml.Name{ + Space: "xmlns", + Local: "xsi", + }, + Value: granteeXMLNS, + }, + Type: t, } } -- 2.45.2