Skip to content

Commit

Permalink
Merge branch 'master' into s3-gw-acl
Browse files Browse the repository at this point in the history
  • Loading branch information
vvarg229 committed Nov 16, 2023
2 parents 7c8b37a + 014a5bf commit 5da7903
Show file tree
Hide file tree
Showing 17 changed files with 178 additions and 60 deletions.
14 changes: 10 additions & 4 deletions api/auth/center.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/nspcc-dev/neofs-s3-gw/api/s3errors"
"github.com/nspcc-dev/neofs-s3-gw/creds/accessbox"
"github.com/nspcc-dev/neofs-s3-gw/creds/tokens"
"github.com/nspcc-dev/neofs-s3-gw/internal/limits"
oid "github.com/nspcc-dev/neofs-sdk-go/object/id"
)

Expand Down Expand Up @@ -104,7 +105,7 @@ func New(neoFS tokens.NeoFS, key *keys.PrivateKey, prefixes []string, config *ca
func (c *center) parseAuthHeader(header string) (*authHeader, error) {
submatches := c.reg.GetSubmatches(header)
if len(submatches) != authHeaderPartsNum {
return nil, s3errors.GetAPIError(s3errors.ErrAuthorizationHeaderMalformed)
return nil, s3errors.GetAPIError(s3errors.ErrCredMalformed)
}

accessKey := strings.Split(submatches["access_key_id"], "0")
Expand Down Expand Up @@ -144,7 +145,7 @@ func (c *center) Authenticate(r *http.Request) (*Box, error) {
if queryValues.Get(AmzAlgorithm) == "AWS4-HMAC-SHA256" {
creds := strings.Split(queryValues.Get(AmzCredential), "/")
if len(creds) != 5 || creds[4] != "aws4_request" {
return nil, fmt.Errorf("bad X-Amz-Credential")
return nil, s3errors.GetAPIError(s3errors.ErrCredMalformed)
}
authHdr = &authHeader{
AccessKeyID: creds[0],
Expand All @@ -159,6 +160,11 @@ func (c *center) Authenticate(r *http.Request) (*Box, error) {
if err != nil {
return nil, fmt.Errorf("couldn't parse X-Amz-Expires: %w", err)
}

if authHdr.Expiration > limits.MaxPreSignedLifetime {
return nil, s3errors.GetAPIError(s3errors.ErrMaximumExpires)
}

Check warning on line 166 in api/auth/center.go

View check run for this annotation

Codecov / codecov/patch

api/auth/center.go#L164-L166

Added lines #L164 - L166 were not covered by tests

signatureDateTimeStr = queryValues.Get(AmzDate)
} else {
authHeaderField := r.Header[AuthorizationHdr]
Expand Down Expand Up @@ -236,9 +242,9 @@ func (c *center) checkFormData(r *http.Request) (*Box, error) {
return nil, ErrNoAuthorizationHeader
}

submatches := c.postReg.GetSubmatches(MultipartFormValue(r, "x-amz-credential"))
submatches := c.postReg.GetSubmatches(MultipartFormValue(r, strings.ToLower(AmzCredential)))
if len(submatches) != 4 {
return nil, s3errors.GetAPIError(s3errors.ErrAuthorizationHeaderMalformed)
return nil, s3errors.GetAPIError(s3errors.ErrCredMalformed)
}

signatureDateTime, err := time.Parse("20060102T150405Z", MultipartFormValue(r, "x-amz-date"))
Expand Down
2 changes: 1 addition & 1 deletion api/auth/center_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func TestAuthHeaderParse(t *testing.T) {
},
{
header: strings.ReplaceAll(defaultHeader, "Signature=2811ccb9e242f41426738fb1f", ""),
err: s3errors.GetAPIError(s3errors.ErrAuthorizationHeaderMalformed),
err: s3errors.GetAPIError(s3errors.ErrCredMalformed),
expected: nil,
},
{
Expand Down
6 changes: 4 additions & 2 deletions api/auth/signer/v4/v4.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ const (
authHeaderSignatureElem = "Signature="
signatureQueryKey = "X-Amz-Signature"

amzCredential = "X-Amz-Credential"

authHeaderPrefix = "AWS4-HMAC-SHA256"
timeFormat = "20060102T150405Z"
shortTimeFormat = "20060102"
Expand Down Expand Up @@ -597,7 +599,7 @@ func (ctx *signingCtx) buildCredentialString() {
ctx.credentialString = buildSigningScope(ctx.Region, ctx.ServiceName, ctx.Time)

if ctx.isPresign {
ctx.Query.Set("X-Amz-Credential", ctx.credValues.AccessKeyID+"/"+ctx.credentialString)
ctx.Query.Set(amzCredential, ctx.credValues.AccessKeyID+"/"+ctx.credentialString)
}
}

Expand Down Expand Up @@ -747,7 +749,7 @@ func (ctx *signingCtx) removePresign() {
ctx.Query.Del("X-Amz-Security-Token")
ctx.Query.Del("X-Amz-Date")
ctx.Query.Del("X-Amz-Expires")
ctx.Query.Del("X-Amz-Credential")
ctx.Query.Del(amzCredential)
ctx.Query.Del("X-Amz-SignedHeaders")
}

Expand Down
44 changes: 29 additions & 15 deletions api/handler/acl.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"encoding/hex"
"encoding/json"
"encoding/xml"
"errors"
stderrors "errors"
"fmt"
"net/http"
Expand Down Expand Up @@ -53,6 +54,10 @@ var actionToOpMap = map[string][]eacl.Operation{
s3ListBucket: readOps,
}

var (
errInvalidPublicKey = errors.New("invalid public key")
)

const (
arnAwsPrefix = "arn:aws:s3:::"
allUsersWildcard = "*"
Expand Down Expand Up @@ -921,23 +926,23 @@ func formRecords(resource *astResource) ([]*eacl.Record, error) {
for _, user := range astOp.Users {
pk, err := keys.NewPublicKeyFromString(user)
if err != nil {
return nil, fmt.Errorf("public key from string: %w", err)
return nil, errInvalidPublicKey
}
targetKeys = append(targetKeys, (ecdsa.PublicKey)(*pk))
}
// Unknown role is used, because it is ignored when keys are set
eacl.AddFormedTarget(record, eacl.RoleUnknown, targetKeys...)
}
if len(resource.Object) != 0 {
if len(resource.Version) != 0 {
var id oid.ID
if err := id.DecodeString(resource.Version); err != nil {
return nil, fmt.Errorf("parse object version (oid): %w", err)
}
record.AddObjectIDFilter(eacl.MatchStringEqual, id)
} else {
record.AddObjectAttributeFilter(eacl.MatchStringEqual, object.AttributeFilePath, resource.Object)
record.AddObjectAttributeFilter(eacl.MatchStringEqual, object.AttributeFilePath, resource.Object)
}

if len(resource.Version) != 0 {
var id oid.ID
if err := id.DecodeString(resource.Version); err != nil {
return nil, fmt.Errorf("parse object version (oid): %w", err)

Check warning on line 943 in api/handler/acl.go

View check run for this annotation

Codecov / codecov/patch

api/handler/acl.go#L943

Added line #L943 was not covered by tests
}
record.AddObjectIDFilter(eacl.MatchStringEqual, id)
}
res = append(res, record)
}
Expand All @@ -960,7 +965,19 @@ func addToList(operations []*astOperation, rec eacl.Record, target eacl.Target)
if found != nil {
if !groupTarget {
for _, key := range target.BinaryKeys() {
found.Users = append(found.Users, hex.EncodeToString(key))
pubKey := hex.EncodeToString(key)
var exist bool

for _, userPubKey := range found.Users {
if userPubKey == pubKey {
exist = true
break

Check warning on line 974 in api/handler/acl.go

View check run for this annotation

Codecov / codecov/patch

api/handler/acl.go#L968-L974

Added lines #L968 - L974 were not covered by tests
}
}

if !exist {
found.Users = append(found.Users, pubKey)
}

Check warning on line 980 in api/handler/acl.go

View check run for this annotation

Codecov / codecov/patch

api/handler/acl.go#L978-L980

Added lines #L978 - L980 were not covered by tests
}
}
} else {
Expand Down Expand Up @@ -1143,10 +1160,7 @@ func aclToAst(acl *AccessControlPolicy, resInfo *resourceInfo) (*ast, error) {

resource := &astResource{resourceInfo: *resInfo}

ops := readOps
if resInfo.IsBucket() {
ops = append(ops, writeOps...)
}
ops := append(readOps, writeOps...)

// Expect to have at least 1 full control grant for owner which is set in
// parseACLHeaders(). If there is no other grants, then user sets private
Expand Down Expand Up @@ -1280,7 +1294,7 @@ func getActions(permission amazonS3Permission, isBucket bool) []string {
if isBucket {
res = []string{s3ListBucket, s3ListBucketVersions, s3ListBucketMultipartUploads, s3PutObject, s3DeleteObject}
} else {
res = []string{s3GetObject, s3GetObjectVersion}
res = []string{s3GetObject, s3GetObjectVersion, s3PutObject, s3DeleteObject}
}
}

Expand Down
59 changes: 44 additions & 15 deletions api/handler/acl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -818,21 +818,21 @@ func TestObjectAclToPolicy(t *testing.T) {
Principal: principal{
CanonicalUser: id,
},
Action: []string{"s3:GetObject", "s3:GetObjectVersion"},
Action: []string{s3GetObject, s3GetObjectVersion, s3PutObject, s3DeleteObject},
Resource: []string{arnAwsPrefix + resInfo.Name()},
},
{
Effect: "Allow",
Principal: principal{
CanonicalUser: id2,
},
Action: []string{"s3:GetObject", "s3:GetObjectVersion"},
Action: []string{s3GetObject, s3GetObjectVersion, s3PutObject, s3DeleteObject},
Resource: []string{arnAwsPrefix + resInfo.Name()},
},
{
Effect: "Allow",
Principal: principal{AWS: allUsersWildcard},
Action: []string{"s3:GetObject", "s3:GetObjectVersion"},
Action: []string{s3GetObject, s3GetObjectVersion},
Resource: []string{arnAwsPrefix + resInfo.Name()},
},
},
Expand Down Expand Up @@ -881,34 +881,53 @@ func TestObjectWithVersionAclToTable(t *testing.T) {
}

func allowedTableForPrivateObject(t *testing.T, key *keys.PrivateKey, resInfo *resourceInfo) *eacl.Table {
var isVersion bool
var objID oid.ID
var zeroObjectID oid.ID

if resInfo.Version != "" {
isVersion = true
err := objID.DecodeString(resInfo.Version)
require.NoError(t, err)
}

expectedTable := eacl.NewTable()

applyFilters := func(r *eacl.Record) {
if resInfo.Object != "" {
r.AddObjectAttributeFilter(eacl.MatchStringEqual, object.AttributeFilePath, resInfo.Object)
}
if !objID.Equals(zeroObjectID) {
r.AddObjectIDFilter(eacl.MatchStringEqual, objID)
}
}

// Order of these loops is important for test.
for i := len(writeOps) - 1; i >= 0; i-- {
op := writeOps[i]
record := getAllowRecord(op, key.PublicKey())

applyFilters(record)
expectedTable.AddRecord(record)
}
for i := len(readOps) - 1; i >= 0; i-- {
op := readOps[i]
record := getAllowRecord(op, key.PublicKey())
if isVersion {
record.AddObjectIDFilter(eacl.MatchStringEqual, objID)
} else {
record.AddObjectAttributeFilter(eacl.MatchStringEqual, object.AttributeFilePath, resInfo.Object)
}

applyFilters(record)
expectedTable.AddRecord(record)
}

for i := len(writeOps) - 1; i >= 0; i-- {
op := writeOps[i]
record := getOthersRecord(op, eacl.ActionDeny)

applyFilters(record)
expectedTable.AddRecord(record)
}
for i := len(readOps) - 1; i >= 0; i-- {
op := readOps[i]
record := getOthersRecord(op, eacl.ActionDeny)
if isVersion {
record.AddObjectIDFilter(eacl.MatchStringEqual, objID)
} else {
record.AddObjectAttributeFilter(eacl.MatchStringEqual, object.AttributeFilePath, resInfo.Object)
}

applyFilters(record)
expectedTable.AddRecord(record)
}

Expand Down Expand Up @@ -1192,6 +1211,16 @@ func TestObjectAclToAst(t *testing.T) {
operations = append(operations, astOp)
}

for _, op := range writeOps {
astOp := &astOperation{Users: []string{
hex.EncodeToString(key.PublicKey().Bytes()),
},
Op: op,
Action: eacl.ActionAllow,
}
operations = append(operations, astOp)
}

expectedAst := &ast{
Resources: []*astResource{
{
Expand Down
11 changes: 6 additions & 5 deletions api/handler/locking_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -436,10 +436,7 @@ func TestObjectLegalHold(t *testing.T) {
putObjectLegalHold(hc, bktName, objName, legalHoldOn)

putObjectLegalHold(hc, bktName, objName, legalHoldOff)
getObjectLegalHold(hc, bktName, objName, legalHoldOff)

// to make sure put hold is an idempotent operation
putObjectLegalHold(hc, bktName, objName, legalHoldOff)
getObjectLegalHold(hc, bktName, objName, legalHoldOn)
}

func getObjectLegalHold(hc *handlerContext, bktName, objName, status string) {
Expand All @@ -451,7 +448,11 @@ func getObjectLegalHold(hc *handlerContext, bktName, objName, status string) {
func putObjectLegalHold(hc *handlerContext, bktName, objName, status string) {
w, r := prepareTestRequest(hc, bktName, objName, &data.LegalHold{Status: status})
hc.Handler().PutObjectLegalHoldHandler(w, r)
assertStatus(hc.t, w, http.StatusOK)
if status == legalHoldOn {
assertStatus(hc.t, w, http.StatusOK)
} else {
assertStatus(hc.t, w, http.StatusNotImplemented)
}
}

func assertLegalHold(t *testing.T, w *httptest.ResponseRecorder, status string) {
Expand Down
16 changes: 16 additions & 0 deletions api/handler/not_support.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,19 @@ func (h *handler) DeleteBucketLifecycleHandler(w http.ResponseWriter, r *http.Re
func (h *handler) DeleteBucketEncryptionHandler(w http.ResponseWriter, r *http.Request) {
h.logAndSendError(w, "not supported", api.GetReqInfo(r.Context()), s3errors.GetAPIError(s3errors.ErrNotSupported))
}

func (h *handler) GetObjectTorrentHandler(w http.ResponseWriter, r *http.Request) {
h.logAndSendError(w, "not supported", api.GetReqInfo(r.Context()), s3errors.GetAPIError(s3errors.ErrNotSupported))
}

func (h *handler) GetBucketPolicyStatusHandler(w http.ResponseWriter, r *http.Request) {
h.logAndSendError(w, "not supported", api.GetReqInfo(r.Context()), s3errors.GetAPIError(s3errors.ErrNotSupported))
}

func (h *handler) PutPublicAccessBlockHandler(w http.ResponseWriter, r *http.Request) {
h.logAndSendError(w, "not supported", api.GetReqInfo(r.Context()), s3errors.GetAPIError(s3errors.ErrNotSupported))
}

func (h *handler) GetPublicAccessBlockHandler(w http.ResponseWriter, r *http.Request) {
h.logAndSendError(w, "not supported", api.GetReqInfo(r.Context()), s3errors.GetAPIError(s3errors.ErrNotSupported))
}
9 changes: 7 additions & 2 deletions api/handler/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ func (h *handler) logAndSendError(w http.ResponseWriter, logText string, reqInfo
}

func transformToS3Error(err error) error {
if _, ok := err.(s3errors.Error); ok {
return err
var s3Err s3errors.Error
if errors.As(err, &s3Err) {
return s3Err
}

if errors.Is(err, layer.ErrAccessDenied) ||
Expand All @@ -43,6 +44,10 @@ func transformToS3Error(err error) error {
return s3errors.GetAPIError(s3errors.ErrUnsupportedMetadata)
}

if errors.Is(err, errInvalidPublicKey) {
return s3errors.GetAPIError(s3errors.ErrInvalidArgument)
}

if errors.Is(err, layer.ErrTooManyObjectForDeletion) {
return s3errors.GetAPIError(s3errors.ErrBadRequest)
}
Expand Down
2 changes: 1 addition & 1 deletion api/layer/cors.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (n *layer) PutBucketCORS(ctx context.Context, p *PutCORSParams) error {
prm := PrmObjectCreate{
Container: p.BktInfo.CID,
Creator: p.BktInfo.Owner,
Payload: p.Reader,
Payload: &buf,

Check warning on line 42 in api/layer/cors.go

View check run for this annotation

Codecov / codecov/patch

api/layer/cors.go#L42

Added line #L42 was not covered by tests
Filepath: p.BktInfo.CORSObjectName(),
CreationTime: TimeNow(ctx),
CopiesNumber: p.CopiesNumber,
Expand Down
5 changes: 1 addition & 4 deletions api/layer/system_object.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,7 @@ func (n *layer) PutLockInfo(ctx context.Context, p *PutLockInfoParams) (err erro
}
lockInfo.SetLegalHold(legalHoldOID)
} else if !newLock.LegalHold.Enabled && lockInfo.IsLegalHoldSet() {
if err = n.objectDelete(ctx, p.ObjVersion.BktInfo, lockInfo.LegalHold()); err != nil {
return fmt.Errorf("couldn't delete lock object '%s' to remove legal hold: %w", lockInfo.LegalHold().EncodeToString(), err)
}
lockInfo.ResetLegalHold()
return s3errors.GetAPIError(s3errors.ErrNotSupported)
}
}

Expand Down
Loading

0 comments on commit 5da7903

Please sign in to comment.