CLDSRV-789: bucket logging check that the source and destination bucket

are in the same account

Author: leif-scality

lib/api/bucketPutLogging.js

@@ -7,6 +7,9 @@ const monitoring = require('../utilities/monitoringHandler');
 const { errorInstances } = require('arsenal');
 const { config } = require('../Config');
 
+const ERROR_MSG_SOURCE_TARGET_BUCKET_OWNER_MISMATCH = 
+    'The owner for the bucket to be logged and the target bucket must be the same.';
+
 function bucketPutLogging(authInfo, request, log, callback) {
     log.debug('processing request', { method: 'bucketPutLogging' });
 
@@ -43,11 +46,16 @@ function bucketPutLogging(authInfo, request, log, callback) {
                 return next(null, bucket);
             }
 
-            return metadata.getBucket(loggingEnabled.TargetBucket, log, err => {
+            return metadata.getBucket(loggingEnabled.TargetBucket, log, (err, targetBucket) => {
                 if (err) {
                     return next(errorInstances.InvalidTargetBucketForLogging.customizeDescription(err.description));
                 }
 
+                if (targetBucket.getOwner() !== bucket.getOwner()) {
+                    return next(errorInstances.InvalidTargetBucketForLogging
+                        .customizeDescription(ERROR_MSG_SOURCE_TARGET_BUCKET_OWNER_MISMATCH));
+                }
+
                 return next(null, bucket);
             });
         },

tests/functional/aws-node-sdk/test/bucket/putBucketLogging.js

@@ -36,6 +36,8 @@ const validLoggingConfigWithGrants = {
     },
 };
 
+const itSkipIfAWS = process.env.AWS_ON_AIR ? it.skip : it;
+
 describe('PUT bucket logging', () => {
     withV4(sigCfg => {
         const bucketUtil = new BucketUtility('default', sigCfg);
@@ -102,7 +104,7 @@ describe('PUT bucket logging', () => {
                 });
             });
 
-            it('should return NotImplemented if TargetGrants is present', done => {
+            itSkipIfAWS('should return NotImplemented if TargetGrants is present', done => {
                 _testPutBucketLoggingError(s3, validLoggingConfigWithGrants, 501, 'NotImplemented', done);
             });
 
@@ -137,7 +139,7 @@ describe('PUT bucket logging', () => {
                 });
             });
 
-            it('should return MethodNotAllowed if user is not bucket owner', done => {
+            itSkipIfAWS('should return MethodNotAllowed if user is not bucket owner', done => {
                 _testPutBucketLoggingError(otherAccountS3, validLoggingConfig, 405, 'MethodNotAllowed', done);
             });
 
@@ -151,6 +153,73 @@ describe('PUT bucket logging', () => {
                     };
                     return _testPutBucketLoggingError(s3, invalidConfig, 400, 'InvalidTargetBucketForLogging', done);
                 });
+
+            it('should allow logging when target bucket is owned by same account', done => {
+                // Both buckets created by same account, should succeed
+                s3.putBucketLogging({
+                    Bucket: bucketName,
+                    BucketLoggingStatus: validLoggingConfig,
+                }, err => {
+                    assert.ifError(err);
+                    // Verify the config was set
+                    s3.getBucketLogging({ Bucket: bucketName }, (err, data) => {
+                        assert.ifError(err);
+                        assert(data.LoggingEnabled);
+                        assert.strictEqual(data.LoggingEnabled.TargetBucket, targetBucket);
+                        return done();
+                    });
+                });
+            });
+        });
+
+        describe('with cross-account target bucket', () => {
+            const otherAccountTargetBucket = 'other-account-target-bucket';
+
+            beforeEach(done => {
+                process.stdout.write('Creating buckets\n');
+                return s3.createBucket({ Bucket: bucketName }, err => {
+                    if (err) {
+                        return done(err);
+                    }
+                    return otherAccountS3.createBucket({ Bucket: otherAccountTargetBucket }, done);
+                });
+            });
+
+            afterEach(done => {
+                process.stdout.write('Deleting buckets\n');
+                Promise.allSettled([
+                    bucketUtil.deleteOne(bucketName),
+                    otherAccountBucketUtility.deleteOne(otherAccountTargetBucket),
+                ]).then(results => {
+                    const errors = results
+                        .filter(r => r.status === 'rejected' && r.reason?.code !== 'NoSuchBucket')
+                        .map(r => r.reason);
+                    if (errors.length > 0) {
+                        return done(errors[0]);
+                    }
+                    return done();
+                });
+            });
+
+            it('should return InvalidTargetBucketForLogging when target bucket is owned by different account', done => {
+                // Try to set logging from first account's bucket to second account's bucket
+                const crossAccountConfig = {
+                    LoggingEnabled: {
+                        TargetBucket: otherAccountTargetBucket,
+                        TargetPrefix: 'logs/',
+                    },
+                };
+                
+                s3.putBucketLogging({
+                    Bucket: bucketName,
+                    BucketLoggingStatus: crossAccountConfig,
+                }, err => {
+                    assert(err, 'Expected error but found none');
+                    assert.strictEqual(err.code, 'InvalidTargetBucketForLogging');
+                    assert.strictEqual(err.statusCode, 400);
+                    done();
+                });
+            });
         });
     });
 });

tests/unit/api/bucketPutLogging.js

@@ -305,4 +305,47 @@ describe('bucketPutLogging API', () => {
             done();
         });
     });
+
+    it('should allow logging when target bucket is owned by requester', done => {
+        // Both buckets are created by authInfo, so this should succeed
+        const loggingXML = createValidLoggingXML(targetBucket);
+        const request = createLoggingRequest(bucketName, loggingXML);
+
+        bucketPutLogging(authInfo, request, log, err => {
+            assert.ifError(err);
+            metadata.getBucket(bucketName, log, (err, bucket) => {
+                assert.ifError(err);
+                const loggingConfig = bucket.getBucketLoggingStatus();
+                assert(loggingConfig);
+                assert.strictEqual(loggingConfig.getLoggingEnabled().TargetBucket, targetBucket);
+                done();
+            });
+        });
+    });
+
+    it('should reject logging when source and target bucket account mismatch', done => {
+        // Create a target bucket with different account
+        const otherAccountTargetBucket = 'other-account-target-bucket';
+        const otherAccountRequest = {
+            bucketName: otherAccountTargetBucket,
+            namespace,
+            headers: { host: `${otherAccountTargetBucket}.s3.amazonaws.com` },
+            url: '/',
+            actionImplicitDenies: false,
+        };
+
+        bucketPut(otherAuthInfo, otherAccountRequest, log, err => {
+            assert.ifError(err);
+
+            // Now try to set logging from authInfo's bucket to otherAuthInfo's bucket
+            const loggingXML = createValidLoggingXML(otherAccountTargetBucket);
+            const request = createLoggingRequest(bucketName, loggingXML);
+
+            bucketPutLogging(authInfo, request, log, err => {
+                assert(err);
+                assert.strictEqual(err.is.InvalidTargetBucketForLogging, true);
+                done();
+            });
+        });
+    });
 });