Skip to content

Commit

Permalink
fix: Changed WORM protection implementation to prevent root/admin use…
Browse files Browse the repository at this point in the history 
…rs to overwrite objects in governance mode or if legal hold is set up
  • Loading branch information
@jonaustin09
jonaustin09 committed May 23, 2024
1 parent f9152ee commit 6fb1020
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 112 deletions.
42 changes: 19 additions & 23 deletions auth/object_lock.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func ParseObjectLegalHoldOutput(status *bool) *types.ObjectLockLegalHold {
}
}

func CheckObjectAccess(ctx context.Context, bucket, userAccess string, objects []string, isAdminOrRoot bool, be backend.Backend) error {
func CheckObjectAccess(ctx context.Context, bucket, userAccess string, objects []string, be backend.Backend) error {
data, err := be.GetObjectLockConfiguration(ctx, bucket)
if err != nil {
if errors.Is(err, s3err.GetAPIError(s3err.ErrObjectLockConfigurationNotFound)) {
Expand Down Expand Up @@ -180,18 +180,16 @@ func CheckObjectAccess(ctx context.Context, bucket, userAccess string, objects [
if retention.RetainUntilDate.After(time.Now()) {
switch retention.Mode {
case types.ObjectLockRetentionModeGovernance:
if !isAdminOrRoot {
policy, err := be.GetBucketPolicy(ctx, bucket)
if errors.Is(err, s3err.GetAPIError(s3err.ErrNoSuchBucketPolicy)) {
return s3err.GetAPIError(s3err.ErrObjectLocked)
}
if err != nil {
return err
}
err = verifyBucketPolicy(policy, userAccess, bucket, obj, BypassGovernanceRetentionAction)
if err != nil {
return s3err.GetAPIError(s3err.ErrObjectLocked)
}
policy, err := be.GetBucketPolicy(ctx, bucket)
if errors.Is(err, s3err.GetAPIError(s3err.ErrNoSuchBucketPolicy)) {
return s3err.GetAPIError(s3err.ErrObjectLocked)
}
if err != nil {
return err
}
err = verifyBucketPolicy(policy, userAccess, bucket, obj, BypassGovernanceRetentionAction)
if err != nil {
return s3err.GetAPIError(s3err.ErrObjectLocked)
}
case types.ObjectLockRetentionModeCompliance:
return s3err.GetAPIError(s3err.ErrObjectLocked)
Expand All @@ -208,7 +206,7 @@ func CheckObjectAccess(ctx context.Context, bucket, userAccess string, objects [
return err
}

if *status && !isAdminOrRoot {
if *status {
return s3err.GetAPIError(s3err.ErrObjectLocked)
}
}
Expand All @@ -225,15 +223,13 @@ func CheckObjectAccess(ctx context.Context, bucket, userAccess string, objects [
if expirationDate.After(time.Now()) {
switch bucketLockConfig.DefaultRetention.Mode {
case types.ObjectLockRetentionModeGovernance:
if !isAdminOrRoot {
policy, err := be.GetBucketPolicy(ctx, bucket)
if err != nil {
return err
}
err = verifyBucketPolicy(policy, userAccess, bucket, "", BypassGovernanceRetentionAction)
if err != nil {
return s3err.GetAPIError(s3err.ErrObjectLocked)
}
policy, err := be.GetBucketPolicy(ctx, bucket)
if err != nil {
return err
}
err = verifyBucketPolicy(policy, userAccess, bucket, "", BypassGovernanceRetentionAction)
if err != nil {
return s3err.GetAPIError(s3err.ErrObjectLocked)
}
case types.ObjectLockRetentionModeCompliance:
return s3err.GetAPIError(s3err.ErrObjectLocked)
Expand Down
6 changes: 3 additions & 3 deletions s3api/controllers/base.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1797,7 +1797,7 @@ func (c S3ApiController) PutActions(ctx *fiber.Ctx) error {
})
}

err = auth.CheckObjectAccess(ctx.Context(), bucket, acct.Access, []string{keyStart}, isRoot || acct.Role == auth.RoleAdmin, c.be)
err = auth.CheckObjectAccess(ctx.Context(), bucket, acct.Access, []string{keyStart}, c.be)
if err != nil {
return SendResponse(ctx, err,
&MetaOpts{
Expand Down Expand Up @@ -2002,7 +2002,7 @@ func (c S3ApiController) DeleteObjects(ctx *fiber.Ctx) error {
})
}

err = auth.CheckObjectAccess(ctx.Context(), bucket, acct.Access, utils.ParseDeleteObjects(dObj.Objects), isRoot || acct.Role == auth.RoleAdmin, c.be)
err = auth.CheckObjectAccess(ctx.Context(), bucket, acct.Access, utils.ParseDeleteObjects(dObj.Objects), c.be)
if err != nil {
return SendResponse(ctx, err,
&MetaOpts{
Expand Down Expand Up @@ -2137,7 +2137,7 @@ func (c S3ApiController) DeleteActions(ctx *fiber.Ctx) error {
})
}

err = auth.CheckObjectAccess(ctx.Context(), bucket, acct.Access, []string{key}, isRoot || acct.Role == auth.RoleAdmin, c.be)
err = auth.CheckObjectAccess(ctx.Context(), bucket, acct.Access, []string{key}, c.be)
if err != nil {
return SendResponse(ctx, err,
&MetaOpts{
Expand Down
4 changes: 0 additions & 4 deletions tests/integration/group-tests.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,9 +372,7 @@ func TestGetObjectLegalHold(s *S3Conf) {

func TestWORMProtection(s *S3Conf) {
WORMProtection_bucket_object_lock_configuration_compliance_mode(s)
WORMProtection_bucket_object_lock_governance_root_overwrite(s)
WORMProtection_object_lock_retention_compliance_root_access_denied(s)
WORMProtection_object_lock_retention_governance_root_overwrite(s)
WORMProtection_object_lock_retention_governance_user_access_denied(s)
WORMProtection_object_lock_legal_hold_user_access_denied(s)
WORMProtection_object_lock_legal_hold_root_overwrite(s)
Expand Down Expand Up @@ -690,9 +688,7 @@ func GetIntTests() IntTests {
"GetObjectLegalHold_unset_config": GetObjectLegalHold_unset_config,
"GetObjectLegalHold_success": GetObjectLegalHold_success,
"WORMProtection_bucket_object_lock_configuration_compliance_mode": WORMProtection_bucket_object_lock_configuration_compliance_mode,
"WORMProtection_bucket_object_lock_governance_root_overwrite": WORMProtection_bucket_object_lock_governance_root_overwrite,
"WORMProtection_object_lock_retention_compliance_root_access_denied": WORMProtection_object_lock_retention_compliance_root_access_denied,
"WORMProtection_object_lock_retention_governance_root_overwrite": WORMProtection_object_lock_retention_governance_root_overwrite,
"WORMProtection_object_lock_retention_governance_user_access_denied": WORMProtection_object_lock_retention_governance_user_access_denied,
"WORMProtection_object_lock_legal_hold_user_access_denied": WORMProtection_object_lock_legal_hold_user_access_denied,
"WORMProtection_object_lock_legal_hold_root_overwrite": WORMProtection_object_lock_legal_hold_root_overwrite,
Expand Down
88 changes: 6 additions & 82 deletions tests/integration/tests.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4465,6 +4465,10 @@ func CreateMultipartUpload_with_object_lock(s *S3Conf) error {
return fmt.Errorf("expected uploaded object lock mode to be %v, instead got %v", types.ObjectLockModeGovernance, resp.ObjectLockMode)
}

if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
return err
}

return nil
}, withLock())
}
Expand Down Expand Up @@ -7682,47 +7686,6 @@ func WORMProtection_bucket_object_lock_configuration_compliance_mode(s *S3Conf)
}, withLock())
}

func WORMProtection_bucket_object_lock_governance_root_overwrite(s *S3Conf) error {
testName := "WORMProtection_bucket_object_lock_governance_root_overwrite"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
var days int32 = 10
object := "my-obj"
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.PutObjectLockConfiguration(ctx, &s3.PutObjectLockConfigurationInput{
Bucket: &bucket,
ObjectLockConfiguration: &types.ObjectLockConfiguration{
ObjectLockEnabled: types.ObjectLockEnabledEnabled,
Rule: &types.ObjectLockRule{
DefaultRetention: &types.DefaultRetention{
Mode: types.ObjectLockRetentionModeGovernance,
Days: &days,
},
},
},
})
cancel()
if err != nil {
return err
}

// create an object
if err := putObjects(s3client, []string{object}, bucket); err != nil {
return err
}

// overwrite the object
if err := putObjects(s3client, []string{object}, bucket); err != nil {
return err
}

if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
return err
}

return nil
}, withLock())
}

func WORMProtection_object_lock_retention_compliance_root_access_denied(s *S3Conf) error {
testName := "WORMProtection_object_lock_retention_compliance_root_access_denied"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
Expand Down Expand Up @@ -7762,46 +7725,6 @@ func WORMProtection_object_lock_retention_compliance_root_access_denied(s *S3Con
}, withLock())
}

func WORMProtection_object_lock_retention_governance_root_overwrite(s *S3Conf) error {
testName := "WORMProtection_object_lock_retention_governance_root_overwrite"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
if err := changeBucketObjectLockStatus(s3client, bucket, true); err != nil {
return err
}

object := "my-obj"

if err := putObjects(s3client, []string{object}, bucket); err != nil {
return err
}

date := time.Now().Add(time.Hour * 3)
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.PutObjectRetention(ctx, &s3.PutObjectRetentionInput{
Bucket: &bucket,
Key: &object,
Retention: &types.ObjectLockRetention{
Mode: types.ObjectLockRetentionModeGovernance,
RetainUntilDate: &date,
},
})
cancel()
if err != nil {
return err
}

if err := putObjects(s3client, []string{object}, bucket); err != nil {
return err
}

if err := changeBucketObjectLockStatus(s3client, bucket, false); err != nil {
return err
}

return nil
}, withLock())
}

func WORMProtection_object_lock_retention_governance_user_access_denied(s *S3Conf) error {
testName := "WORMProtection_object_lock_retention_governance_user_access_denied"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
Expand Down Expand Up @@ -7940,7 +7863,8 @@ func WORMProtection_object_lock_legal_hold_root_overwrite(s *S3Conf) error {
return err
}

if err := putObjects(s3client, []string{object}, bucket); err != nil {
err = putObjects(s3client, []string{object}, bucket)
if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrObjectLocked)); err != nil {
return err
}

Expand Down

0 comments on commit 6fb1020

Please sign in to comment.