From e76f66a0d0878dd9647cdef2250a205206d2b98b Mon Sep 17 00:00:00 2001
From: "changlin.shi" <changlin.shi@ly.com>
Date: Thu, 13 Oct 2022 17:12:06 +0800
Subject: [PATCH] add acl validation for s3 bucket read api

---
 weed/s3api/s3api_acp.go                   | 38 +++++++++++++++++++++
 weed/s3api/s3api_bucket_handlers.go       | 41 ++++++++---------------
 weed/s3api/s3api_objects_list_handlers.go | 12 +++++++
 3 files changed, 64 insertions(+), 27 deletions(-)

diff --git a/weed/s3api/s3api_acp.go b/weed/s3api/s3api_acp.go
index 7a76c2a67..065457ef9 100644
--- a/weed/s3api/s3api_acp.go
+++ b/weed/s3api/s3api_acp.go
@@ -1,8 +1,10 @@
 package s3api
 
 import (
+	"github.com/seaweedfs/seaweedfs/weed/glog"
 	"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
 	"github.com/seaweedfs/seaweedfs/weed/s3api/s3account"
+	"github.com/seaweedfs/seaweedfs/weed/s3api/s3acl"
 	"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
 	"net/http"
 )
@@ -27,3 +29,39 @@ func (s3a *S3ApiServer) checkAccessByOwnership(r *http.Request, bucket string) s
 	}
 	return s3err.ErrAccessDenied
 }
+
+// Check Bucket/BucketAcl Read related access
+// includes:
+// - HeadBucketHandler
+// - GetBucketAclHandler
+// - ListObjectsV1Handler
+// - ListObjectsV2Handler
+func (s3a *S3ApiServer) checkAccessForReadBucket(r *http.Request, bucket, aclAction string) (*BucketMetaData, s3err.ErrorCode) {
+	bucketMetadata, errCode := s3a.bucketRegistry.GetBucketMetadata(bucket)
+	if errCode != s3err.ErrNone {
+		return nil, errCode
+	}
+
+	if bucketMetadata.ObjectOwnership == s3_constants.OwnershipBucketOwnerEnforced {
+		return bucketMetadata, s3err.ErrNone
+	}
+
+	accountId := s3acl.GetAccountId(r)
+	if accountId == s3account.AccountAdmin.Id || accountId == *bucketMetadata.Owner.ID {
+		return bucketMetadata, s3err.ErrNone
+	}
+
+	if len(bucketMetadata.Acl) > 0 {
+		reqGrants := s3acl.DetermineReqGrants(accountId, aclAction)
+		for _, bucketGrant := range bucketMetadata.Acl {
+			for _, reqGrant := range reqGrants {
+				if s3acl.GrantEquals(bucketGrant, reqGrant) {
+					return bucketMetadata, s3err.ErrNone
+				}
+			}
+		}
+	}
+
+	glog.V(3).Infof("acl denied! request account id: %s", accountId)
+	return nil, s3err.ErrAccessDenied
+}
diff --git a/weed/s3api/s3api_bucket_handlers.go b/weed/s3api/s3api_bucket_handlers.go
index 9e215db9e..6f94fd027 100644
--- a/weed/s3api/s3api_bucket_handlers.go
+++ b/weed/s3api/s3api_bucket_handlers.go
@@ -200,6 +200,11 @@ func (s3a *S3ApiServer) HeadBucketHandler(w http.ResponseWriter, r *http.Request
 	bucket, _ := s3_constants.GetBucketAndObject(r)
 	glog.V(3).Infof("HeadBucketHandler %s", bucket)
 
+	_, errorCode := s3a.checkAccessForReadBucket(r, bucket, s3_constants.PermissionRead)
+	if errorCode != s3err.ErrNone {
+		s3err.WriteErrorResponse(w, r, errorCode)
+		return
+	}
 	if entry, err := s3a.getEntry(s3a.option.BucketsPath, bucket); entry == nil || err == filer_pb.ErrNotFound {
 		s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket)
 		return
@@ -245,37 +250,19 @@ func (s3a *S3ApiServer) GetBucketAclHandler(w http.ResponseWriter, r *http.Reque
 	bucket, _ := s3_constants.GetBucketAndObject(r)
 	glog.V(3).Infof("GetBucketAclHandler %s", bucket)
 
-	if err := s3a.checkBucket(r, bucket); err != s3err.ErrNone {
-		s3err.WriteErrorResponse(w, r, err)
+	bucketMetadata, errorCode := s3a.checkAccessForReadBucket(r, bucket, s3_constants.PermissionReadAcp)
+	if s3err.ErrNone != errorCode {
+		s3err.WriteErrorResponse(w, r, errorCode)
 		return
 	}
 
-	response := AccessControlPolicy{}
-	for _, ident := range s3a.iam.identities {
-		if len(ident.Credentials) == 0 {
-			continue
-		}
-		for _, action := range ident.Actions {
-			if !action.overBucket(bucket) || action.getPermission() == "" {
-				continue
-			}
-			id := ident.Credentials[0].AccessKey
-			if response.Owner.DisplayName == "" && action.isOwner(bucket) && len(ident.Credentials) > 0 {
-				response.Owner.DisplayName = ident.Name
-				response.Owner.ID = id
-			}
-			response.AccessControlList.Grant = append(response.AccessControlList.Grant, Grant{
-				Grantee: Grantee{
-					ID:          id,
-					DisplayName: ident.Name,
-					Type:        "CanonicalUser",
-					XMLXSI:      "CanonicalUser",
-					XMLNS:       "http://www.w3.org/2001/XMLSchema-instance"},
-				Permission: action.getPermission(),
-			})
-		}
+	acp := &s3.PutBucketAclInput{
+		AccessControlPolicy: &s3.AccessControlPolicy{
+			Grants: bucketMetadata.Acl,
+			Owner:  bucketMetadata.Owner,
+		},
 	}
-	writeSuccessResponseXML(w, r, response)
+	s3err.WriteAwsXMLResponse(w, r, http.StatusOK, acp)
 }
 
 // GetBucketLifecycleConfigurationHandler Get Bucket Lifecycle configuration
diff --git a/weed/s3api/s3api_objects_list_handlers.go b/weed/s3api/s3api_objects_list_handlers.go
index 620969fd6..97ba83d19 100644
--- a/weed/s3api/s3api_objects_list_handlers.go
+++ b/weed/s3api/s3api_objects_list_handlers.go
@@ -41,6 +41,12 @@ func (s3a *S3ApiServer) ListObjectsV2Handler(w http.ResponseWriter, r *http.Requ
 	bucket, _ := s3_constants.GetBucketAndObject(r)
 	glog.V(3).Infof("ListObjectsV2Handler %s", bucket)
 
+	_, errCode := s3a.checkAccessForReadBucket(r, bucket, s3_constants.PermissionRead)
+	if errCode != s3err.ErrNone {
+		s3err.WriteErrorResponse(w, r, errCode)
+		return
+	}
+
 	originalPrefix, continuationToken, startAfter, delimiter, _, maxKeys := getListObjectsV2Args(r.URL.Query())
 
 	if maxKeys < 0 {
@@ -97,6 +103,12 @@ func (s3a *S3ApiServer) ListObjectsV1Handler(w http.ResponseWriter, r *http.Requ
 	bucket, _ := s3_constants.GetBucketAndObject(r)
 	glog.V(3).Infof("ListObjectsV1Handler %s", bucket)
 
+	_, errCode := s3a.checkAccessForReadBucket(r, bucket, s3_constants.PermissionRead)
+	if errCode != s3err.ErrNone {
+		s3err.WriteErrorResponse(w, r, errCode)
+		return
+	}
+
 	originalPrefix, marker, delimiter, maxKeys := getListObjectsV1Args(r.URL.Query())
 
 	if maxKeys < 0 {