[#301] Support GetBucketPolicyStatus

Signed-off-by: Denis Kirillov <d.kirillov@yadro.com>

CHANGELOG.md

@@ -27,6 +27,7 @@ This document outlines major changes between releases.
 - Support `proxy` contract (#287)
 - Authmate: support custom attributes (#292)
 - Add new `reconnect_interval` config param (#291)
+- Support `GetBucketPolicyStatus` (#301)
 
 ### Changed
 - Generalise config param `use_default_xmlns_for_complete_multipart` to `use_default_xmlns` so that use default xmlns for all requests (#221)

api/errors/errors.go

@@ -91,6 +91,7 @@ const (
 	ErrBucketNotEmpty
 	ErrAllAccessDisabled
 	ErrMalformedPolicy
+	ErrMalformedPolicyNotPrincipal
 	ErrMissingFields
 	ErrMissingCredTag
 	ErrCredMalformed
@@ -665,6 +666,12 @@ var errorCodes = errorCodeMap{
 		Description:    "Policy has invalid resource.",
 		HTTPStatusCode: http.StatusBadRequest,
 	},
+	ErrMalformedPolicyNotPrincipal: {
+		ErrCode:        ErrMalformedPolicyNotPrincipal,
+		Code:           "MalformedPolicy",
+		Description:    "Allow with NotPrincipal is not allowed.",
+		HTTPStatusCode: http.StatusBadRequest,
+	},
 	ErrMissingFields: {
 		ErrCode:        ErrMissingFields,
 		Code:           "MissingFields",

api/handler/acl.go

@@ -650,6 +650,48 @@ func (h *handler) PutObjectACLHandler(w http.ResponseWriter, r *http.Request) {
 	w.WriteHeader(http.StatusOK)
 }
 
+func (h *handler) GetBucketPolicyStatusHandler(w http.ResponseWriter, r *http.Request) {
+	reqInfo := middleware.GetReqInfo(r.Context())
+
+	bktInfo, err := h.getBucketAndCheckOwner(r, reqInfo.BucketName)
+	if err != nil {
+		h.logAndSendError(w, "could not get bucket info", reqInfo, err)
+		return
+	}
+
+	jsonPolicy, err := h.ape.GetBucketPolicy(reqInfo.Namespace, bktInfo.CID)
+	if err != nil {
+		if strings.Contains(err.Error(), "not found") {
+			err = fmt.Errorf("%w: %s", errors.GetAPIError(errors.ErrNoSuchBucketPolicy), err.Error())
+		}
+		h.logAndSendError(w, "failed to get policy from storage", reqInfo, err)
+		return
+	}
+
+	var bktPolicy engineiam.Policy
+	if err = json.Unmarshal(jsonPolicy, &bktPolicy); err != nil {
+		h.logAndSendError(w, "could not parse bucket policy", reqInfo, err)
+		return
+	}
+
+	policyStatus := &PolicyStatus{
+		IsPublic: PolicyStatusIsPublicFalse,
+	}
+
+	for _, st := range bktPolicy.Statement {
+		// https://docs.aws.amazon.com/AmazonS3/latest/userguide/access-control-block-public-access.html#access-control-block-public-access-policy-status
+		if _, ok := st.Principal[engineiam.Wildcard]; ok {
+			policyStatus.IsPublic = PolicyStatusIsPublicTrue
+			break
+		}
+	}
+
+	if err = middleware.EncodeToResponse(w, policyStatus); err != nil {
+		h.logAndSendError(w, "encode and write response", reqInfo, err)
+		return
+	}
+}
+
 func (h *handler) GetBucketPolicyHandler(w http.ResponseWriter, r *http.Request) {
 	reqInfo := middleware.GetReqInfo(r.Context())
 
@@ -731,6 +773,11 @@ func (h *handler) PutBucketPolicyHandler(w http.ResponseWriter, r *http.Request)
 			return
 		}
 
+		if len(stat.NotPrincipal) != 0 && stat.Effect == engineiam.AllowEffect {
+			h.logAndSendError(w, "invalid NotPrincipal", reqInfo, errors.GetAPIError(errors.ErrMalformedPolicyNotPrincipal))
+			return
+		}
+
 		for _, resource := range stat.Resource {
 			if reqInfo.BucketName != strings.Split(strings.TrimPrefix(resource, arnAwsPrefix), "/")[0] {
 				h.logAndSendError(w, "policy resource mismatched bucket", reqInfo, errors.GetAPIError(errors.ErrMalformedPolicy))

api/handler/acl_test.go

@@ -7,6 +7,7 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"encoding/json"
+	"encoding/xml"
 	"fmt"
 	"io"
 	"net/http"
@@ -1467,6 +1468,41 @@ func TestBucketPolicy(t *testing.T) {
 	require.Equal(t, newPolicy, bktPolicy)
 }
 
+func TestBucketPolicyStatus(t *testing.T) {
+	hc := prepareHandlerContext(t)
+	bktName := "bucket-for-policy"
+
+	createTestBucket(hc, bktName)
+
+	getBucketPolicy(hc, bktName, s3errors.ErrNoSuchBucketPolicy)
+
+	newPolicy := engineiam.Policy{
+		Statement: []engineiam.Statement{{
+			NotPrincipal: engineiam.Principal{engineiam.Wildcard: {}},
+			Effect:       engineiam.AllowEffect,
+			Action:       engineiam.Action{"s3:PutObject"},
+			Resource:     engineiam.Resource{arnAwsPrefix + bktName + "/*"},
+		}},
+	}
+
+	putBucketPolicy(hc, bktName, newPolicy, s3errors.ErrMalformedPolicyNotPrincipal)
+
+	newPolicy.Statement[0].NotPrincipal = nil
+	newPolicy.Statement[0].Principal = map[engineiam.PrincipalType][]string{engineiam.Wildcard: {}}
+	putBucketPolicy(hc, bktName, newPolicy)
+	bktPolicyStatus := getBucketPolicyStatus(hc, bktName)
+	require.True(t, PolicyStatusIsPublicTrue == bktPolicyStatus.IsPublic)
+
+	key, err := keys.NewPrivateKey()
+	require.NoError(t, err)
+	hc.Handler().frostfsid.(*frostfsidMock).data["devenv"] = key.PublicKey()
+
+	newPolicy.Statement[0].Principal = map[engineiam.PrincipalType][]string{engineiam.AWSPrincipalType: {"arn:aws:iam:::user/devenv"}}
+	putBucketPolicy(hc, bktName, newPolicy)
+	bktPolicyStatus = getBucketPolicyStatus(hc, bktName)
+	require.True(t, PolicyStatusIsPublicFalse == bktPolicyStatus.IsPublic)
+}
+
 func TestBucketPolicyUnmarshal(t *testing.T) {
 	for _, tc := range []struct {
 		name   string
@@ -1557,6 +1593,22 @@ func getBucketPolicy(hc *handlerContext, bktName string, errCode ...s3errors.Err
 	return policy
 }
 
+func getBucketPolicyStatus(hc *handlerContext, bktName string, errCode ...s3errors.ErrorCode) PolicyStatus {
+	w, r := prepareTestRequest(hc, bktName, "", nil)
+	hc.Handler().GetBucketPolicyStatusHandler(w, r)
+
+	var policyStatus PolicyStatus
+	if len(errCode) == 0 {
+		assertStatus(hc.t, w, http.StatusOK)
+		err := xml.NewDecoder(w.Result().Body).Decode(&policyStatus)
+		require.NoError(hc.t, err)
+	} else {
+		assertS3Error(hc.t, w, s3errors.GetAPIError(errCode[0]))
+	}
+
+	return policyStatus
+}
+
 func putBucketPolicy(hc *handlerContext, bktName string, bktPolicy engineiam.Policy, errCode ...s3errors.ErrorCode) {
 	body, err := json.Marshal(bktPolicy)
 	require.NoError(hc.t, err)

api/handler/handlers_test.go

@@ -4,8 +4,10 @@ import (
 	"bytes"
 	"context"
 	"crypto/rand"
+	"encoding/hex"
 	"encoding/xml"
 	"errors"
+	"fmt"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -177,10 +179,11 @@ func prepareHandlerContextBase(t *testing.T, cacheCfg *layer.CachesConfig) *hand
 		defaultPolicy: pp,
 	}
 	h := &handler{
-		log: l,
-		obj: layer.NewLayer(l, tp, layerCfg),
-		cfg: cfg,
-		ape: newAPEMock(),
+		log:       l,
+		obj:       layer.NewLayer(l, tp, layerCfg),
+		cfg:       cfg,
+		ape:       newAPEMock(),
+		frostfsid: newFrostfsIDMock(),
 	}
 
 	return &handlerContext{
@@ -301,6 +304,32 @@ func (a *apeMock) SaveACLChains(ns string, chains []*chain.Chain) error {
 	return nil
 }
 
+type frostfsidMock struct {
+	data map[string]*keys.PublicKey
+}
+
+func newFrostfsIDMock() *frostfsidMock {
+	return &frostfsidMock{data: map[string]*keys.PublicKey{}}
+}
+
+func (f *frostfsidMock) GetUserAddress(account, user string) (string, error) {
+	res, ok := f.data[account+user]
+	if !ok {
+		return "", fmt.Errorf("not found")
+	}
+
+	return res.Address(), nil
+}
+
+func (f *frostfsidMock) GetUserKey(account, user string) (string, error) {
+	res, ok := f.data[account+user]
+	if !ok {
+		return "", fmt.Errorf("not found")
+	}
+
+	return hex.EncodeToString(res.Bytes()), nil
+}
+
 func createTestBucket(hc *handlerContext, bktName string) *data.BucketInfo {
 	info := createBucket(hc, bktName)
 	return info.BktInfo

api/handler/response.go

@@ -55,6 +55,19 @@ type Bucket struct {
 	CreationDate string // time string of format "2006-01-02T15:04:05.000Z"
 }
 
+// PolicyStatus contains status of bucket policy.
+type PolicyStatus struct {
+	XMLName  xml.Name             `xml:"http://s3.amazonaws.com/doc/2006-03-01/ PolicyStatus" json:"-"`
+	IsPublic PolicyStatusIsPublic `xml:"IsPublic"`
+}
+
+type PolicyStatusIsPublic string
+
+const (
+	PolicyStatusIsPublicFalse = "FALSE"
+	PolicyStatusIsPublicTrue  = "TRUE"
+)
+
 // AccessControlPolicy contains ACL.
 type AccessControlPolicy struct {
 	XMLName           xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ AccessControlPolicy" json:"-"`

api/middleware/constants.go

@@ -9,6 +9,7 @@ const (
 	HeadBucketOperation                = "HeadBucket"
 	ListMultipartUploadsOperation      = "ListMultipartUploads"
 	GetBucketLocationOperation         = "GetBucketLocation"
+	GetBucketPolicyStatusOperation     = "GetBucketPolicyStatus"
 	GetBucketPolicyOperation           = "GetBucketPolicy"
 	GetBucketLifecycleOperation        = "GetBucketLifecycle"
 	GetBucketEncryptionOperation       = "GetBucketEncryption"
@@ -77,6 +78,7 @@ const (
 const (
 	UploadsQuery        = "uploads"
 	LocationQuery       = "location"
+	PolicyStatusQuery   = "policyStatus"
 	PolicyQuery         = "policy"
 	LifecycleQuery      = "lifecycle"
 	EncryptionQuery     = "encryption"

api/router.go

@@ -37,6 +37,7 @@ type (
 		PutObjectHandler(http.ResponseWriter, *http.Request)
 		DeleteObjectHandler(http.ResponseWriter, *http.Request)
 		GetBucketLocationHandler(http.ResponseWriter, *http.Request)
+		GetBucketPolicyStatusHandler(http.ResponseWriter, *http.Request)
 		GetBucketPolicyHandler(http.ResponseWriter, *http.Request)
 		GetBucketLifecycleHandler(http.ResponseWriter, *http.Request)
 		GetBucketEncryptionHandler(http.ResponseWriter, *http.Request)
@@ -230,6 +231,9 @@ func bucketRouter(h Handler, log *zap.Logger) chi.Router {
 			Add(NewFilter().
 				Queries(s3middleware.LocationQuery).
 				Handler(named(s3middleware.GetBucketLocationOperation, h.GetBucketLocationHandler))).
+			Add(NewFilter().
+				Queries(s3middleware.PolicyStatusQuery).
+				Handler(named(s3middleware.GetBucketPolicyStatusOperation, h.GetBucketPolicyStatusHandler))).
 			Add(NewFilter().
 				Queries(s3middleware.PolicyQuery).
 				Handler(named(s3middleware.GetBucketPolicyOperation, h.GetBucketPolicyHandler))).

api/router_mock_test.go

@@ -186,6 +186,11 @@ func (h *handlerMock) GetBucketLocationHandler(http.ResponseWriter, *http.Reques
 	panic("implement me")
 }
 
+func (h *handlerMock) GetBucketPolicyStatusHandler(http.ResponseWriter, *http.Request) {
+	//TODO implement me
+	panic("implement me")
+}
+
 func (h *handlerMock) GetBucketPolicyHandler(http.ResponseWriter, *http.Request) {
 	//TODO implement me
 	panic("implement me")

docs/aws_s3_compat.md

@@ -207,17 +207,38 @@ See also `GetObject` and other method parameters.
 
 ## Policy and replication
 
-|    | Method                  | Comments                    |
-|----|-------------------------|-----------------------------|
-| 🔵 | DeleteBucketPolicy      |                             |
-| 🔵 | DeleteBucketReplication |                             |
-| 🔵 | DeletePublicAccessBlock |                             |
-| 🟡 | GetBucketPolicy         | See ACL limitations         |
-| 🔵 | GetBucketPolicyStatus   |                             |
-| 🔵 | GetBucketReplication    |                             |
-| 🟢 | PostPolicyBucket        | Upload file using POST form |
-| 🟡 | PutBucketPolicy         | See ACL limitations         |
-| 🔵 | PutBucketReplication    |                             |
+Bucket policy has the following limitations
+* Supports only AWS principals in format `arn:aws:iam::<namespace>:user/<user>` or wildcard `*`.
+* No complex conditions (only conditions for groups now supported)
+
+Simple valid policy example:
+```json
+{
+	"Version": "2012-10-17",
+	"Statement": [{
+		"Principal": {"AWS": ["arn:aws:iam::111122223333:role/JohnDoe"]},
+		"Effect": "Allow",
+		"Action": ["s3:GetObject","s3:GetObjectVersion"],
+		"Resource": ["arn:aws:s3:::DOC-EXAMPLE-BUCKET/*"]
+	}]
+}
+```
+
+Bucket policy status determines using the following scheme:
+* If policy has statement with principal that is wildcard (`*`) then policy is considered as public
+
+
+|     | Method                  | Comments                    |
+|-----|-------------------------|-----------------------------|
+| 🟡  | DeleteBucketPolicy      | See Policy limitations      |
+| 🔵  | DeleteBucketReplication |                             |
+| 🔵  | DeletePublicAccessBlock |                             |
+| 🟡  | GetBucketPolicy         | See Policy limitations      |
+| 🟡  | GetBucketPolicyStatus   |                             |
+| 🔵  | GetBucketReplication    |                             |
+| 🟢  | PostPolicyBucket        | Upload file using POST form |
+| 🟡  | PutBucketPolicy         | See Policy limitations      |
+| 🔵  | PutBucketReplication    |                             |
 
 ## Request payment