From a3338dbd34466e01cedffaf0ad9f4f05ed0c4da6 Mon Sep 17 00:00:00 2001
From: Ben McClelland <ben.mcclelland@versity.com>
Date: Tue, 11 Feb 2025 10:24:04 -0800
Subject: [PATCH] fix: return default bucket acl if none exists

We were trying to parse a non existing acl and returning an
internal server error due to invalid json acl data.

If the bucket acl does not exist, return a default acl with the
root account as the owner.

This fixes #1060, but does not address the invalid acl format
from s3cmd reported in #963.
---
 auth/acl.go                         | 17 ++++++++++++++---
 s3api/controllers/base.go           |  2 +-
 s3api/middlewares/acl-parser.go     |  5 +++++
 s3api/middlewares/authentication.go |  1 +
 s3api/middlewares/presign-auth.go   |  2 ++
 5 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/auth/acl.go b/auth/acl.go
index 80dec11c1..784296707 100644
--- a/auth/acl.go
+++ b/auth/acl.go
@@ -193,14 +193,25 @@ func ParseACL(data []byte) (ACL, error) {
 	return acl, nil
 }
 
-func ParseACLOutput(data []byte) (GetBucketAclOutput, error) {
+func ParseACLOutput(data []byte, owner string) (GetBucketAclOutput, error) {
+	grants := []Grant{}
+
+	if len(data) == 0 {
+		return GetBucketAclOutput{
+			Owner: &types.Owner{
+				ID: &owner,
+			},
+			AccessControlList: AccessControlList{
+				Grants: grants,
+			},
+		}, nil
+	}
+
 	var acl ACL
 	if err := json.Unmarshal(data, &acl); err != nil {
 		return GetBucketAclOutput{}, fmt.Errorf("parse acl: %w", err)
 	}
 
-	grants := []Grant{}
-
 	for _, elem := range acl.Grantees {
 		acs := elem.Access
 		grants = append(grants, Grant{
diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go
index 0b23ac5d1..02947bf6f 100644
--- a/s3api/controllers/base.go
+++ b/s3api/controllers/base.go
@@ -921,7 +921,7 @@ func (c S3ApiController) ListActions(ctx *fiber.Ctx) error {
 				})
 		}
 
-		res, err := auth.ParseACLOutput(data)
+		res, err := auth.ParseACLOutput(data, parsedAcl.Owner)
 		return SendXMLResponse(ctx, res, err,
 			&MetaOpts{
 				Logger:      c.logger,
diff --git a/s3api/middlewares/acl-parser.go b/s3api/middlewares/acl-parser.go
index 593d54cc1..cdc1bdaa7 100644
--- a/s3api/middlewares/acl-parser.go
+++ b/s3api/middlewares/acl-parser.go
@@ -74,6 +74,11 @@ func AclParser(be backend.Backend, logger s3log.AuditLogger, readonly bool) fibe
 			return controllers.SendResponse(ctx, err, &controllers.MetaOpts{Logger: logger})
 		}
 
+		// if owner is not set, set default owner to root account
+		if parsedAcl.Owner == "" {
+			parsedAcl.Owner = ctx.Locals("rootAccess").(string)
+		}
+
 		ctx.Locals("parsedAcl", parsedAcl)
 		return ctx.Next()
 	}
diff --git a/s3api/middlewares/authentication.go b/s3api/middlewares/authentication.go
index 90ce2196b..2291e7099 100644
--- a/s3api/middlewares/authentication.go
+++ b/s3api/middlewares/authentication.go
@@ -76,6 +76,7 @@ func VerifyV4Signature(root RootUserConfig, iam auth.IAMService, logger s3log.Au
 		}
 
 		ctx.Locals("isRoot", authData.Access == root.Access)
+		ctx.Locals("rootAccess", root.Access)
 
 		account, err := acct.getAccount(authData.Access)
 		if err == auth.ErrNoSuchUser {
diff --git a/s3api/middlewares/presign-auth.go b/s3api/middlewares/presign-auth.go
index 18f694280..aee10a712 100644
--- a/s3api/middlewares/presign-auth.go
+++ b/s3api/middlewares/presign-auth.go
@@ -43,6 +43,8 @@ func VerifyPresignedV4Signature(root RootUserConfig, iam auth.IAMService, logger
 		}
 
 		ctx.Locals("isRoot", authData.Access == root.Access)
+		ctx.Locals("rootAccess", root.Access)
+
 		account, err := acct.getAccount(authData.Access)
 		if err == auth.ErrNoSuchUser {
 			return sendResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidAccessKeyID), logger, mm)