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

Closed
KirillovDenis wants to merge 1 commit from KirillovDenis/bugfix/681-frostfs-fix_acl_parsing into master
3 changed files with 130 additions and 53 deletions

View file

@ -65,13 +65,10 @@ const (
aclRead AWSACL = "READ" aclRead AWSACL = "READ"
) )
// GranteeType is aws grantee permission type constants.
type GranteeType string
const ( const (
acpCanonicalUser GranteeType = "CanonicalUser" acpCanonicalUser = "CanonicalUser"
acpAmazonCustomerByEmail GranteeType = "AmazonCustomerByEmail" acpAmazonCustomerByEmail = "AmazonCustomerByEmail"
acpGroup GranteeType = "Group" acpGroup = "Group"
) )
type bucketPolicy struct { 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 { } else if err = xml.NewDecoder(r.Body).Decode(list); err != nil {
h.logAndSendError(w, "could not parse bucket acl", reqInfo, errors.GetAPIError(errors.ErrMalformedXML)) h.logAndSendError(w, "could not parse bucket acl", reqInfo, errors.GetAPIError(errors.ErrMalformedXML))
return return
} else {
// workaround to decode owner ID
list.Owner.ID = hex.EncodeToString(key.Bytes())
} }
resInfo := &resourceInfo{Bucket: reqInfo.BucketName} 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 { } else if err = xml.NewDecoder(r.Body).Decode(list); err != nil {
h.logAndSendError(w, "could not parse bucket acl", reqInfo, errors.GetAPIError(errors.ErrMalformedXML)) h.logAndSendError(w, "could not parse bucket acl", reqInfo, errors.GetAPIError(errors.ErrMalformedXML))
return return
} else {
// workaround to decode owner ID
list.Owner.ID = hex.EncodeToString(key.Bytes())
} }
resInfo := &resourceInfo{ resInfo := &resourceInfo{
@ -469,7 +472,7 @@ func parseACLHeaders(header http.Header, key *keys.PublicKey) (*AccessControlPol
Grantee: &Grantee{ Grantee: &Grantee{
ID: hex.EncodeToString(key.Bytes()), ID: hex.EncodeToString(key.Bytes()),
DisplayName: key.Address(), DisplayName: key.Address(),
Type: acpCanonicalUser, Type: formGranteeType(acpCanonicalUser),
}, },
Permission: aclFullControl, Permission: aclFullControl,
}} }}
@ -509,7 +512,7 @@ func addGrantees(list []*Grant, headers http.Header, hdr string) ([]*Grant, erro
} }
for _, grantee := range grantees { 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") return nil, stderrors.New("unsupported grantee type")
} }
@ -559,17 +562,17 @@ func formGrantee(granteeType, value string) (*Grantee, error) {
case "id": case "id":
return &Grantee{ return &Grantee{
ID: value, ID: value,
Type: acpCanonicalUser, Type: formGranteeType(acpCanonicalUser),
}, nil }, nil
case "uri": case "uri":
return &Grantee{ return &Grantee{
URI: value, URI: value,
Type: acpGroup, Type: formGranteeType(acpGroup),
}, nil }, nil
case "emailAddress": case "emailAddress":
return &Grantee{ return &Grantee{
EmailAddress: value, EmailAddress: value,
Type: acpAmazonCustomerByEmail, Type: formGranteeType(acpAmazonCustomerByEmail),
}, nil }, nil
} }
// do not return grantee type to avoid sensitive data logging (#489) // 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{ acp.AccessControlList = append(acp.AccessControlList, &Grant{
Grantee: &Grantee{ Grantee: &Grantee{
URI: allUsersGroup, URI: allUsersGroup,
Type: acpGroup, Type: formGranteeType(acpGroup),
}, },
Permission: aclFullControl, Permission: aclFullControl,
}) })
@ -593,7 +596,7 @@ func addPredefinedACP(acp *AccessControlPolicy, cannedACL string) (*AccessContro
acp.AccessControlList = append(acp.AccessControlList, &Grant{ acp.AccessControlList = append(acp.AccessControlList, &Grant{
Grantee: &Grantee{ Grantee: &Grantee{
URI: allUsersGroup, URI: allUsersGroup,
Type: acpGroup, Type: formGranteeType(acpGroup),
}, },
Permission: aclRead, Permission: aclRead,
}) })
@ -1173,12 +1176,12 @@ func aclToAst(acl *AccessControlPolicy, resInfo *resourceInfo) (*ast, error) {
} }
for _, grant := range acl.AccessControlList { 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") return nil, stderrors.New("unsupported grantee type")
} }
var groupGrantee bool var groupGrantee bool
if grant.Grantee.Type == acpGroup { if grant.Grantee.matchType(acpGroup) {
groupGrantee = true groupGrantee = true
} else if grant.Grantee.ID == acl.Owner.ID { } else if grant.Grantee.ID == acl.Owner.ID {
continue continue
@ -1212,12 +1215,12 @@ func aclToPolicy(acl *AccessControlPolicy, resInfo *resourceInfo) (*bucketPolicy
} }
for _, grant := range acl.AccessControlList { 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") return nil, stderrors.New("unsupported grantee type")
} }
user := grant.Grantee.ID user := grant.Grantee.ID
if grant.Grantee.Type == acpGroup { if grant.Grantee.matchType(acpGroup) {
user = allUsersWildcard user = allUsersWildcard
} else if user == acl.Owner.ID { } else if user == acl.Owner.ID {
continue continue
@ -1377,10 +1380,10 @@ func (h *handler) encodeObjectACL(bucketACL *layer.BucketACL, bucketName, object
var grantee *Grantee var grantee *Grantee
if key == allUsersGroup { if key == allUsersGroup {
grantee = NewGrantee(acpGroup) grantee = NewGrantee(formGranteeType(acpGroup))
grantee.URI = allUsersGroup grantee.URI = allUsersGroup
} else { } else {
grantee = NewGrantee(acpCanonicalUser) grantee = NewGrantee(formGranteeType(acpCanonicalUser))
grantee.ID = key grantee.ID = key
} }
@ -1453,7 +1456,7 @@ func bucketACLToTable(acp *AccessControlPolicy, resInfo *resourceInfo) (*eacl.Ta
} }
func getRecordFunction(grantee *Grantee) (getRecordFunc, error) { func getRecordFunction(grantee *Grantee) (getRecordFunc, error) {
switch grantee.Type { switch grantee.TypeString() {
case acpAmazonCustomerByEmail: case acpAmazonCustomerByEmail:
case acpCanonicalUser: case acpCanonicalUser:
pk, err := keys.NewPublicKeyFromString(grantee.ID) pk, err := keys.NewPublicKeyFromString(grantee.ID)
@ -1473,7 +1476,7 @@ func getRecordFunction(grantee *Grantee) (getRecordFunc, error) {
func isValidGrant(grant *Grant) bool { func isValidGrant(grant *Grant) bool {
return (grant.Permission == aclFullControl || grant.Permission == aclRead || grant.Permission == aclWrite) && 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 { func getAllowRecord(op eacl.Operation, pk *keys.PublicKey) *eacl.Record {

View file

@ -8,6 +8,7 @@ import (
"crypto/sha256" "crypto/sha256"
"encoding/hex" "encoding/hex"
"encoding/json" "encoding/json"
"encoding/xml"
"fmt" "fmt"
"io" "io"
"net/http" "net/http"
@ -724,13 +725,13 @@ func TestBucketAclToPolicy(t *testing.T) {
AccessControlList: []*Grant{{ AccessControlList: []*Grant{{
Grantee: &Grantee{ Grantee: &Grantee{
URI: allUsersGroup, URI: allUsersGroup,
Type: acpGroup, Type: formGranteeType(acpGroup),
}, },
Permission: aclRead, Permission: aclRead,
}, { }, {
Grantee: &Grantee{ Grantee: &Grantee{
ID: id2, ID: id2,
Type: acpCanonicalUser, Type: formGranteeType(acpCanonicalUser),
}, },
Permission: aclWrite, Permission: aclWrite,
}}, }},
@ -790,19 +791,19 @@ func TestObjectAclToPolicy(t *testing.T) {
AccessControlList: []*Grant{{ AccessControlList: []*Grant{{
Grantee: &Grantee{ Grantee: &Grantee{
ID: id, ID: id,
Type: acpCanonicalUser, Type: formGranteeType(acpCanonicalUser),
}, },
Permission: aclFullControl, Permission: aclFullControl,
}, { }, {
Grantee: &Grantee{ Grantee: &Grantee{
ID: id2, ID: id2,
Type: acpCanonicalUser, Type: formGranteeType(acpCanonicalUser),
}, },
Permission: aclFullControl, Permission: aclFullControl,
}, { }, {
Grantee: &Grantee{ Grantee: &Grantee{
URI: allUsersGroup, URI: allUsersGroup,
Type: acpGroup, Type: formGranteeType(acpGroup),
}, },
Permission: aclRead, Permission: aclRead,
}}, }},
@ -859,7 +860,7 @@ func TestObjectWithVersionAclToTable(t *testing.T) {
AccessControlList: []*Grant{{ AccessControlList: []*Grant{{
Grantee: &Grantee{ Grantee: &Grantee{
ID: id, ID: id,
Type: acpCanonicalUser, Type: formGranteeType(acpCanonicalUser),
}, },
Permission: aclFullControl, Permission: aclFullControl,
}}, }},
@ -980,13 +981,13 @@ func TestParseCannedACLHeaders(t *testing.T) {
Grantee: &Grantee{ Grantee: &Grantee{
ID: id, ID: id,
DisplayName: address, DisplayName: address,
Type: acpCanonicalUser, Type: formGranteeType(acpCanonicalUser),
}, },
Permission: aclFullControl, Permission: aclFullControl,
}, { }, {
Grantee: &Grantee{ Grantee: &Grantee{
URI: allUsersGroup, URI: allUsersGroup,
Type: acpGroup, Type: formGranteeType(acpGroup),
}, },
Permission: aclRead, Permission: aclRead,
}}, }},
@ -1021,37 +1022,37 @@ func TestParseACLHeaders(t *testing.T) {
Grantee: &Grantee{ Grantee: &Grantee{
ID: id, ID: id,
DisplayName: address, DisplayName: address,
Type: acpCanonicalUser, Type: formGranteeType(acpCanonicalUser),
}, },
Permission: aclFullControl, Permission: aclFullControl,
}, { }, {
Grantee: &Grantee{ Grantee: &Grantee{
ID: "user1", ID: "user1",
Type: acpCanonicalUser, Type: formGranteeType(acpCanonicalUser),
}, },
Permission: aclFullControl, Permission: aclFullControl,
}, { }, {
Grantee: &Grantee{ Grantee: &Grantee{
URI: allUsersGroup, URI: allUsersGroup,
Type: acpGroup, Type: formGranteeType(acpGroup),
}, },
Permission: aclRead, Permission: aclRead,
}, { }, {
Grantee: &Grantee{ Grantee: &Grantee{
ID: "user2", ID: "user2",
Type: acpCanonicalUser, Type: formGranteeType(acpCanonicalUser),
}, },
Permission: aclRead, Permission: aclRead,
}, { }, {
Grantee: &Grantee{ Grantee: &Grantee{
ID: "user2", ID: "user2",
Type: acpCanonicalUser, Type: formGranteeType(acpCanonicalUser),
}, },
Permission: aclWrite, Permission: aclWrite,
}, { }, {
Grantee: &Grantee{ Grantee: &Grantee{
ID: "user3", ID: "user3",
Type: acpCanonicalUser, Type: formGranteeType(acpCanonicalUser),
}, },
Permission: aclWrite, Permission: aclWrite,
}}, }},
@ -1112,13 +1113,13 @@ func TestBucketAclToTable(t *testing.T) {
AccessControlList: []*Grant{{ AccessControlList: []*Grant{{
Grantee: &Grantee{ Grantee: &Grantee{
URI: allUsersGroup, URI: allUsersGroup,
Type: acpGroup, Type: formGranteeType(acpGroup),
}, },
Permission: aclRead, Permission: aclRead,
}, { }, {
Grantee: &Grantee{ Grantee: &Grantee{
ID: id2, ID: id2,
Type: acpCanonicalUser, Type: formGranteeType(acpCanonicalUser),
}, },
Permission: aclWrite, Permission: aclWrite,
}}, }},
@ -1169,13 +1170,13 @@ func TestObjectAclToAst(t *testing.T) {
AccessControlList: []*Grant{{ AccessControlList: []*Grant{{
Grantee: &Grantee{ Grantee: &Grantee{
ID: id, ID: id,
Type: acpCanonicalUser, Type: formGranteeType(acpCanonicalUser),
}, },
Permission: aclFullControl, Permission: aclFullControl,
}, { }, {
Grantee: &Grantee{ Grantee: &Grantee{
ID: id2, ID: id2,
Type: acpCanonicalUser, Type: formGranteeType(acpCanonicalUser),
}, },
Permission: aclRead, Permission: aclRead,
}, },
@ -1238,13 +1239,13 @@ func TestBucketAclToAst(t *testing.T) {
{ {
Grantee: &Grantee{ Grantee: &Grantee{
ID: id2, ID: id2,
Type: acpCanonicalUser, Type: formGranteeType(acpCanonicalUser),
}, },
Permission: aclWrite, Permission: aclWrite,
}, { }, {
Grantee: &Grantee{ Grantee: &Grantee{
URI: allUsersGroup, URI: allUsersGroup,
Type: acpGroup, Type: formGranteeType(acpGroup),
}, },
Permission: aclRead, Permission: aclRead,
}, },
@ -1436,3 +1437,47 @@ func putBucketACL(t *testing.T, tc *handlerContext, bktName string, box *accessb
tc.Handler().PutBucketACLHandler(w, r) tc.Handler().PutBucketACLHandler(w, r)
assertStatus(t, w, http.StatusOK) 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)
}

View file

@ -71,19 +71,48 @@ type Grant struct {
// Grantee is info about access rights of some actor. // Grantee is info about access rights of some actor.
type Grantee struct { type Grantee struct {
XMLName xml.Name `xml:"Grantee"` XMLName xml.Name `xml:"Grantee"`
XMLNS string `xml:"xmlns:xsi,attr"` XMLNS xml.Attr `xml:"xsi,attr"`
ID string `xml:"ID,omitempty"` ID string `xml:"ID,omitempty"`
DisplayName string `xml:"DisplayName,omitempty"` DisplayName string `xml:"DisplayName,omitempty"`
EmailAddress string `xml:"EmailAddress,omitempty"` EmailAddress string `xml:"EmailAddress,omitempty"`
URI string `xml:"URI,omitempty"` URI string `xml:"URI,omitempty"`
Type GranteeType `xml:"xsi:type,attr"` Type xml.Attr `xml:"type,attr"`
} }
// NewGrantee creates new grantee using workaround func (g Grantee) matchType(t string) bool {
// https://github.com/golang/go/issues/9519#issuecomment-252196382 return g.Type.Value == t
func NewGrantee(t GranteeType) *Grantee { }
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{ return &Grantee{
XMLNS: "http://www.w3.org/2001/XMLSchema-instance", XMLName: xml.Name{
Local: "Grantee",
},
XMLNS: xml.Attr{
Name: xml.Name{
Space: "xmlns",
Local: "xsi",
},
Value: granteeXMLNS,
},
Type: t, Type: t,
} }
} }