From 23ea76f22e3ebaabf105c5ef0759de26cf1ea7a4 Mon Sep 17 00:00:00 2001 From: Matt Skillman Date: Fri, 14 Aug 2020 08:38:17 -0400 Subject: [PATCH 1/7] wip; addition of initial logic --- .../cloudtrailBucketAccessLogging.js | 59 +++++++++++++++++-- .../cloudtrailBucketAccessLogging.spec.js | 0 2 files changed, 53 insertions(+), 6 deletions(-) create mode 100644 plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.spec.js diff --git a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js index 2c3b213ad8..dd9506f7ae 100644 --- a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js +++ b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js @@ -1,6 +1,22 @@ var async = require('async'); var helpers = require('../../../helpers/aws'); +function bucketIsInAccount(allBuckets, targetBucketName) { + for (let i in allBuckets) { + let bucket = allBuckets[i]; + if (bucket.Name === targetBucketName) { + return true; // target bucket present in account + } + } + return false; // not present in account +} + +function bucketExists(err) { + return !(err && + err.code && + err.code === 'NoSuchBucket'); +} + module.exports = { title: 'CloudTrail Bucket Access Logging', category: 'CloudTrail', @@ -8,7 +24,7 @@ module.exports = { more_info: 'CloudTrail buckets should utilize access logging for an additional layer of auditing. If the log files are deleted or modified in any way, the additional access logs can help determine who made the changes.', recommended_action: 'Enable access logging on the CloudTrail bucket from the S3 console', link: 'http://docs.aws.amazon.com/AmazonS3/latest/UG/ManagingBucketLogging.html', - apis: ['CloudTrail:describeTrails', 'S3:getBucketLogging'], + apis: ['CloudTrail:describeTrails', 'S3:getBucketLogging', 'S3:listBuckets'], compliance: { hipaa: 'Access logging for CloudTrail helps ensure strict integrity controls, ' + 'verifying that the audit logs for the AWS environment are not modified.', @@ -16,14 +32,23 @@ module.exports = { 'in which cardholder data is present. CloudTrail bucket access logging ' + 'helps audit the bucket in which these logs are stored.' }, + settings: { + ignore_bucket_not_in_account: { + name: 'Ignore CloudTrail Buckets Not in Account', + description: 'enable to ignore cloudtrail buckets that are not in the account', + regex: '^(true|false)$', // string true or boolean true to enable, string false or boolean false to disable + default: true + }, + }, run: function(cache, settings, callback) { + var config = { + ignore_bucket_not_in_account: settings.ignore_bucket_not_in_account || this.settings.ignore_bucket_not_in_account.default + }; var results = []; var source = {}; var regions = helpers.regions(settings); - async.each(regions.cloudtrail, function(region, rcb){ - var describeTrails = helpers.addSource(cache, source, ['cloudtrail', 'describeTrails', region]); @@ -47,15 +72,37 @@ module.exports = { var s3Region = helpers.defaultRegion(settings); + const listBuckets = helpers.addSource(cache, source, ['s3', 'listBuckets', s3Region]); + if (!listBuckets) return rcb(null, results, source); + if (listBuckets.err || !listBuckets.data) { + helpers.addResult(results, 3, `Unable to query for S3 buckets: ${helpers.addError(listBuckets)}`); + return rcb(null, results, source); + } + var getBucketLogging = helpers.addSource(cache, source, ['s3', 'getBucketLogging', s3Region, trail.S3BucketName]); if (!getBucketLogging || getBucketLogging.err || !getBucketLogging.data) { - helpers.addResult(results, 3, - 'Error querying for bucket policy for bucket: ' + trail.S3BucketName + ': ' + helpers.addError(getBucketLogging), + if (!bucketExists(getBucketLogging.err)) { + helpers.addResult(results, 2, + 'Bucket: ' + trail.S3BucketName + ' does not exist' , + region, 'arn:aws:s3:::' + trail.S3BucketName); + + return cb(); + } + else if (config.ignore_bucket_not_in_account && !bucketIsInAccount(listBuckets.data, trail.S3BucketName)) { + helpers.addResult(results, 0, + 'Bucket: ' + trail.S3BucketName + ' is not in account', region, 'arn:aws:s3:::' + trail.S3BucketName); - return cb(); + return cb(); + } else { + helpers.addResult(results, 3, + 'Error querying for bucket policy for bucket: ' + trail.S3BucketName + ': ' + helpers.addError(getBucketLogging), + region, 'arn:aws:s3:::' + trail.S3BucketName); + + return cb(); + } } if (getBucketLogging && diff --git a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.spec.js b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.spec.js new file mode 100644 index 0000000000..e69de29bb2 From 782798fadea7cd8ab1826755631af506598b9d41 Mon Sep 17 00:00:00 2001 From: Matt Skillman Date: Fri, 14 Aug 2020 11:38:05 -0400 Subject: [PATCH 2/7] addition of test case for cloudtrailBucketAccessLogging.js --- .../cloudtrailBucketAccessLogging.js | 2 +- .../cloudtrailBucketAccessLogging.spec.js | 199 ++++++++++++++++++ 2 files changed, 200 insertions(+), 1 deletion(-) diff --git a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js index dd9506f7ae..0038e64166 100644 --- a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js +++ b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js @@ -82,7 +82,7 @@ module.exports = { var getBucketLogging = helpers.addSource(cache, source, ['s3', 'getBucketLogging', s3Region, trail.S3BucketName]); - if (!getBucketLogging || getBucketLogging.err || !getBucketLogging.data) { + if (!getBucketLogging || getBucketLogging.err) { // data will be {} if logging is disabled. if (!bucketExists(getBucketLogging.err)) { helpers.addResult(results, 2, 'Bucket: ' + trail.S3BucketName + ' does not exist' , diff --git a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.spec.js b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.spec.js index e69de29bb2..22dc2341e7 100644 --- a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.spec.js +++ b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.spec.js @@ -0,0 +1,199 @@ +var assert = require('assert'); +var expect = require('chai').expect; +var eks = require('./cloudtrailBucketAccessLogging'); // TODO everything is just copied over, need to rehaul +const createCache = (descTrailsData, getBuckLogData, listBuckData, buckName) => { + let to_return = { + cloudtrail: { + describeTrails: { + 'us-east-1': { + data: descTrailsData + } + }, + }, + s3: { + getBucketLogging: { + 'us-east-1': {} + }, + listBuckets: { + 'us-east-1': { + data: listBuckData + } + }, + } + }; + to_return.s3.getBucketLogging['us-east-1'][buckName] = getBuckLogData; + return to_return +}; + +describe('cloudtrailBucketAccessLogging', function () { + describe('run', function () { + it('should PASS if CloudTrail logging bucket has access logging enabled', function (done) { + const callback = (err, results) => { + expect(results.length).to.equal(1); + expect(results[0].status).to.equal(0); + done(); + }; + + const cache = createCache( + [ + { + "Name": "delete-me-ms", + "S3BucketName": "delete-me-ueoeuaou-ms", + "IncludeGlobalServiceEvents": true, + "IsMultiRegionTrail": true, + "HomeRegion": "us-east-1", + "TrailARN": "arn:aws:cloudtrail:us-east-1:000000000000:trail/delete-me-ms", + "LogFileValidationEnabled": true, + "HasCustomEventSelectors": true, + "HasInsightSelectors": false, + "IsOrganizationTrail": false + }, + ], + { + data: { + "LoggingEnabled": { + "TargetPrefix": "foo", + "TargetBucket": "delete-me-ueoeuaou-ms" + } + }, + }, + [ + { + "Name": "delete-me-ueoeuaou-ms", + "CreationDate": "2020-06-30T14:29:53.000Z" + }, + ], + 'delete-me-ueoeuaou-ms' + ); + + eks.run(cache, {}, callback); + }); + + it('should WARN if CloudTrail logging bucket has access logging disabled', function (done) { + const callback = (err, results) => { + expect(results.length).to.equal(1); + expect(results[0].status).to.equal(1); + done(); + }; + + const cache = createCache( + [ + { + "Name": "delete-me-ms", + "S3BucketName": "delete-me-ueoeuaou-ms", + "IncludeGlobalServiceEvents": true, + "IsMultiRegionTrail": true, + "HomeRegion": "us-east-1", + "TrailARN": "arn:aws:cloudtrail:us-east-1:000000000000:trail/delete-me-ms", + "LogFileValidationEnabled": true, + "HasCustomEventSelectors": true, + "HasInsightSelectors": false, + "IsOrganizationTrail": false + }, + ], + { + data: {}, + }, + [ + { + "Name": "delete-me-ueoeuaou-ms", + "CreationDate": "2020-06-30T14:29:53.000Z" + }, + ], + 'delete-me-ueoeuaou-ms' + ); + + eks.run(cache, {}, callback); + }); + + it('should FAIL if CloudTrail logging bucket has access logging enabled but bucket does not exist', function (done) { + const callback = (err, results) => { + expect(results.length).to.equal(1); + expect(results[0].status).to.equal(2); + done(); + }; + + const cache = createCache( + [ + { + "Name": "delete-me-ms", + "S3BucketName": "delete-me-ueoeuaou-ms", + "IncludeGlobalServiceEvents": true, + "IsMultiRegionTrail": true, + "HomeRegion": "us-east-1", + "TrailARN": "arn:aws:cloudtrail:us-east-1:000000000000:trail/delete-me-ms", + "LogFileValidationEnabled": true, + "HasCustomEventSelectors": true, + "HasInsightSelectors": false, + "IsOrganizationTrail": false + }, + ], + { + data: {}, + err: { + "message": "The specified bucket does not exist", + "code": "NoSuchBucket", + "region": null, + "time": "2020-08-13T17:17:02.086Z", + "requestId": "0000000000000000", + "extendedRequestId": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "statusCode": 404, + "retryable": false, + "retryDelay": 52.88593440570173 + }, + }, + [ + { + "Name": "delete-me-ueoeuaou-ms", + "CreationDate": "2020-06-30T14:29:53.000Z" + }, + ], + 'delete-me-ueoeuaou-ms' + ); + + eks.run(cache, {}, callback); + }); + + it('should PASS if CloudTrail logging bucket has access logging enabled and bucket is another account', function (done) { + const callback = (err, results) => { + expect(results.length).to.equal(1); + expect(results[0].status).to.equal(0); + done(); + }; + + const cache = createCache( + [ + { + "Name": "delete-me-ms", + "S3BucketName": "delete-me-ueoeuaou-ms", + "IncludeGlobalServiceEvents": true, + "IsMultiRegionTrail": true, + "HomeRegion": "us-east-1", + "TrailARN": "arn:aws:cloudtrail:us-east-1:000000000000:trail/delete-me-ms", + "LogFileValidationEnabled": true, + "HasCustomEventSelectors": true, + "HasInsightSelectors": false, + "IsOrganizationTrail": false + }, + ], + { + data: { + "LoggingEnabled": { + "TargetPrefix": "foo", + "TargetBucket": "delete-me-ueoeuaou-ms" + }, + }, + }, + [ + { + "Name": "not-delete-me-ueoeuaou-ms", + "CreationDate": "2020-06-30T14:29:53.000Z" + }, + ], + 'delete-me-ueoeuaou-ms' + ); + + eks.run(cache, {}, callback); + }); + }) +}); \ No newline at end of file From 4a61ff48de96e847038c3bef70ba6833fef59bda Mon Sep 17 00:00:00 2001 From: Matt Skillman Date: Fri, 14 Aug 2020 11:40:16 -0400 Subject: [PATCH 3/7] ignore bucket not in account setting defaults to false now --- plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js | 2 +- plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.spec.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js index 0038e64166..e321d8553c 100644 --- a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js +++ b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js @@ -37,7 +37,7 @@ module.exports = { name: 'Ignore CloudTrail Buckets Not in Account', description: 'enable to ignore cloudtrail buckets that are not in the account', regex: '^(true|false)$', // string true or boolean true to enable, string false or boolean false to disable - default: true + default: false }, }, diff --git a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.spec.js b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.spec.js index 22dc2341e7..fc3bdc5248 100644 --- a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.spec.js +++ b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.spec.js @@ -154,7 +154,7 @@ describe('cloudtrailBucketAccessLogging', function () { eks.run(cache, {}, callback); }); - it('should PASS if CloudTrail logging bucket has access logging enabled and bucket is another account', function (done) { + it('should PASS if CloudTrail logging bucket has access logging enabled and bucket is another account, with ignore_bucket_not_in_account enabled', function (done) { const callback = (err, results) => { expect(results.length).to.equal(1); expect(results[0].status).to.equal(0); @@ -193,7 +193,7 @@ describe('cloudtrailBucketAccessLogging', function () { 'delete-me-ueoeuaou-ms' ); - eks.run(cache, {}, callback); + eks.run(cache, {ignore_bucket_not_in_account: true}, callback); }); }) }); \ No newline at end of file From e6282a5bd590eff1dd0c4a646ad8e5fc67e07b83 Mon Sep 17 00:00:00 2001 From: Matt Skillman Date: Fri, 14 Aug 2020 12:10:26 -0400 Subject: [PATCH 4/7] BucketDelete plugin updated with new setting --- .../cloudtrailBucketAccessLogging.js | 1 + .../cloudtrailBucketAccessLogging.spec.js | 2 +- .../aws/cloudtrail/cloudtrailBucketDelete.js | 60 ++++- .../cloudtrail/cloudtrailBucketDelete.spec.js | 254 ++++++++++++++++++ 4 files changed, 311 insertions(+), 6 deletions(-) create mode 100644 plugins/aws/cloudtrail/cloudtrailBucketDelete.spec.js diff --git a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js index e321d8553c..e789a4e957 100644 --- a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js +++ b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js @@ -45,6 +45,7 @@ module.exports = { var config = { ignore_bucket_not_in_account: settings.ignore_bucket_not_in_account || this.settings.ignore_bucket_not_in_account.default }; + if (config.ignore_bucket_not_in_account === 'false') config.ignore_bucket_not_in_account = false; var results = []; var source = {}; var regions = helpers.regions(settings); diff --git a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.spec.js b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.spec.js index fc3bdc5248..f7a56b3eb6 100644 --- a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.spec.js +++ b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.spec.js @@ -1,6 +1,6 @@ var assert = require('assert'); var expect = require('chai').expect; -var eks = require('./cloudtrailBucketAccessLogging'); // TODO everything is just copied over, need to rehaul +var eks = require('./cloudtrailBucketAccessLogging'); const createCache = (descTrailsData, getBuckLogData, listBuckData, buckName) => { let to_return = { cloudtrail: { diff --git a/plugins/aws/cloudtrail/cloudtrailBucketDelete.js b/plugins/aws/cloudtrail/cloudtrailBucketDelete.js index c43a1b01d4..ce9908392c 100644 --- a/plugins/aws/cloudtrail/cloudtrailBucketDelete.js +++ b/plugins/aws/cloudtrail/cloudtrailBucketDelete.js @@ -1,6 +1,22 @@ var async = require('async'); var helpers = require('../../../helpers/aws'); +function bucketIsInAccount(allBuckets, targetBucketName) { + for (let i in allBuckets) { + let bucket = allBuckets[i]; + if (bucket.Name === targetBucketName) { + return true; // target bucket present in account + } + } + return false; // not present in account +} + +function bucketExists(err) { + return !(err && + err.code && + err.code === 'NoSuchBucket'); +} + module.exports = { title: 'CloudTrail Bucket Delete Policy', category: 'CloudTrail', @@ -8,15 +24,27 @@ module.exports = { more_info: 'To provide additional security, CloudTrail logging buckets should require an MFA token to delete objects', recommended_action: 'Enable MFA delete on the CloudTrail bucket', link: 'http://docs.aws.amazon.com/AmazonS3/latest/dev/Versioning.html#MultiFactorAuthenticationDelete', - apis: ['CloudTrail:describeTrails', 'S3:getBucketVersioning'], + apis: ['CloudTrail:describeTrails', 'S3:getBucketVersioning', 'S3:listBuckets'], compliance: { hipaa: 'An MFA delete policy helps ensure that individuals attempting to ' + 'delete CloudTrail logs have verified their identity. HIPAA requires ' + 'strict access controls for users modifying the environments in which ' + 'HIPAA data is stored.' }, + settings: { + ignore_bucket_not_in_account: { + name: 'Ignore CloudTrail Buckets Not in Account', + description: 'enable to ignore cloudtrail buckets that are not in the account', + regex: '^(true|false)$', // string true or boolean true to enable, string false or boolean false to disable + default: false + }, + }, run: function(cache, settings, callback) { + var config = { + ignore_bucket_not_in_account: settings.ignore_bucket_not_in_account || this.settings.ignore_bucket_not_in_account.default + }; + if (config.ignore_bucket_not_in_account === 'false') config.ignore_bucket_not_in_account = false; var results = []; var source = {}; var regions = helpers.regions(settings); @@ -46,15 +74,37 @@ module.exports = { var s3Region = helpers.defaultRegion(settings); + const listBuckets = helpers.addSource(cache, source, ['s3', 'listBuckets', s3Region]); + if (!listBuckets) return rcb(null, results, source); + if (listBuckets.err || !listBuckets.data) { + helpers.addResult(results, 3, `Unable to query for S3 buckets: ${helpers.addError(listBuckets)}`); + return rcb(null, results, source); + } + var getBucketVersioning = helpers.addSource(cache, source, ['s3', 'getBucketVersioning', s3Region, trail.S3BucketName]); - if (!getBucketVersioning || getBucketVersioning.err || !getBucketVersioning.data) { - helpers.addResult(results, 3, - 'Error querying for bucket policy for bucket: ' + trail.S3BucketName + ': ' + helpers.addError(getBucketVersioning), + if (!getBucketVersioning || getBucketVersioning.err || !getBucketVersioning.data) { // data is {} if disabled. this assumes other plugin checks to see if enabled. + if (!bucketExists(getBucketVersioning.err)) { + helpers.addResult(results, 2, + 'Bucket: ' + trail.S3BucketName + ' does not exist' , + region, 'arn:aws:s3:::' + trail.S3BucketName); + + return cb(); + } + else if (config.ignore_bucket_not_in_account && !bucketIsInAccount(listBuckets.data, trail.S3BucketName)) { + helpers.addResult(results, 0, + 'Bucket: ' + trail.S3BucketName + ' is not in account', region, 'arn:aws:s3:::' + trail.S3BucketName); - return cb(); + return cb(); + } else { + helpers.addResult(results, 3, + 'Error querying for bucket policy for bucket: ' + trail.S3BucketName + ': ' + helpers.addError(getBucketVersioning), + region, 'arn:aws:s3:::' + trail.S3BucketName); + + return cb(); + } } if (getBucketVersioning.data.MFADelete && diff --git a/plugins/aws/cloudtrail/cloudtrailBucketDelete.spec.js b/plugins/aws/cloudtrail/cloudtrailBucketDelete.spec.js new file mode 100644 index 0000000000..6250803c38 --- /dev/null +++ b/plugins/aws/cloudtrail/cloudtrailBucketDelete.spec.js @@ -0,0 +1,254 @@ +var assert = require('assert'); +var expect = require('chai').expect; +var eks = require('./cloudtrailBucketDelete'); +const createCache = (descTrailsData, getBuckVerData, listBuckData, buckName) => { + let to_return = { + cloudtrail: { + describeTrails: { + 'us-east-1': { + data: descTrailsData + } + }, + }, + s3: { + getBucketVersioning: { + 'us-east-1': {} + }, + listBuckets: { + 'us-east-1': { + data: listBuckData + } + }, + } + }; + to_return.s3.getBucketVersioning['us-east-1'][buckName] = getBuckVerData; + return to_return +}; + +describe('cloudtrailBucketAccessLogging', function () { + describe('run', function () { + it('should PASS if CloudTrail logging bucket has MFADelete enabled', function (done) { + const callback = (err, results) => { + expect(results.length).to.equal(1); + expect(results[0].status).to.equal(0); + done(); + }; + + const cache = createCache( + [ + { + "Name": "delete-me-ms", + "S3BucketName": "delete-me-ueoeuaou-ms", + "IncludeGlobalServiceEvents": true, + "IsMultiRegionTrail": true, + "HomeRegion": "us-east-1", + "TrailARN": "arn:aws:cloudtrail:us-east-1:000000000000:trail/delete-me-ms", + "LogFileValidationEnabled": true, + "HasCustomEventSelectors": true, + "HasInsightSelectors": false, + "IsOrganizationTrail": false + }, + ], + { + data: { + "Status": "Enabled", + "MFADelete": "Enabled" + }, + }, + [ + { + "Name": "delete-me-ueoeuaou-ms", + "CreationDate": "2020-06-30T14:29:53.000Z" + }, + ], + 'delete-me-ueoeuaou-ms' + ); + + eks.run(cache, {}, callback); + }); + + it('should WARN if CloudTrail logging bucket has MFADelete disabled', function (done) { + const callback = (err, results) => { + expect(results.length).to.equal(1); + expect(results[0].status).to.equal(1); + done(); + }; + + const cache = createCache( + [ + { + "Name": "delete-me-ms", + "S3BucketName": "delete-me-ueoeuaou-ms", + "IncludeGlobalServiceEvents": true, + "IsMultiRegionTrail": true, + "HomeRegion": "us-east-1", + "TrailARN": "arn:aws:cloudtrail:us-east-1:000000000000:trail/delete-me-ms", + "LogFileValidationEnabled": true, + "HasCustomEventSelectors": true, + "HasInsightSelectors": false, + "IsOrganizationTrail": false + }, + ], + { + data: { + "Status": "Enabled", + "MFADelete": "Disabled" + }, + }, + [ + { + "Name": "delete-me-ueoeuaou-ms", + "CreationDate": "2020-06-30T14:29:53.000Z" + }, + ], + 'delete-me-ueoeuaou-ms' + ); + + eks.run(cache, {}, callback); + }); + + it('should FAIL if CloudTrail logging bucket does not exist', function (done) { + const callback = (err, results) => { + expect(results.length).to.equal(1); + expect(results[0].status).to.equal(2); + done(); + }; + + const cache = createCache( + [ + { + "Name": "delete-me-ms", + "S3BucketName": "delete-me-ueoeuaou-ms", + "IncludeGlobalServiceEvents": true, + "IsMultiRegionTrail": true, + "HomeRegion": "us-east-1", + "TrailARN": "arn:aws:cloudtrail:us-east-1:000000000000:trail/delete-me-ms", + "LogFileValidationEnabled": true, + "HasCustomEventSelectors": true, + "HasInsightSelectors": false, + "IsOrganizationTrail": false + }, + ], + { + data: {}, + err: { + "message": "The specified bucket does not exist", + "code": "NoSuchBucket", + "region": null, + "time": "2020-08-13T17:17:02.086Z", + "requestId": "0000000000000000", + "extendedRequestId": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "statusCode": 404, + "retryable": false, + "retryDelay": 52.88593440570173 + }, + }, + [ + { + "Name": "delete-me-ueoeuaou-ms", + "CreationDate": "2020-06-30T14:29:53.000Z" + }, + ], + 'delete-me-ueoeuaou-ms' + ); + + eks.run(cache, {}, callback); + }); + + it('should PASS if CloudTrail logging bucket is in another account, with ignore_bucket_not_in_account enabled', function (done) { + const callback = (err, results) => { + expect(results.length).to.equal(1); + expect(results[0].status).to.equal(0); + done(); + }; + + const cache = createCache( + [ + { + "Name": "delete-me-ms", + "S3BucketName": "delete-me-ueoeuaou-ms", + "IncludeGlobalServiceEvents": true, + "IsMultiRegionTrail": true, + "HomeRegion": "us-east-1", + "TrailARN": "arn:aws:cloudtrail:us-east-1:000000000000:trail/delete-me-ms", + "LogFileValidationEnabled": true, + "HasCustomEventSelectors": true, + "HasInsightSelectors": false, + "IsOrganizationTrail": false + }, + ], + { + data: {}, + err: { + "message": "Access Denied", + "code": "AccessDenied", + "region": null, + "time": "2020-08-13T17:17:02.086Z", + "requestId": "0000000000000000", + "extendedRequestId": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "statusCode": 404, + "retryable": false, + "retryDelay": 52.88593440570173 + }, + }, + [ + { + "Name": "not-delete-me-ueoeuaou-ms", + "CreationDate": "2020-06-30T14:29:53.000Z" + }, + ], + 'delete-me-ueoeuaou-ms' + ); + + eks.run(cache, {ignore_bucket_not_in_account: true}, callback); + }); + + it('should UNKNOWN if CloudTrail logging bucket is in another account, with ignore_bucket_not_in_account disabled', function (done) { + const callback = (err, results) => { + expect(results.length).to.equal(1); + expect(results[0].status).to.equal(3); + done(); + }; + + const cache = createCache( + [ + { + "Name": "delete-me-ms", + "S3BucketName": "delete-me-ueoeuaou-ms", + "IncludeGlobalServiceEvents": true, + "IsMultiRegionTrail": true, + "HomeRegion": "us-east-1", + "TrailARN": "arn:aws:cloudtrail:us-east-1:000000000000:trail/delete-me-ms", + "LogFileValidationEnabled": true, + "HasCustomEventSelectors": true, + "HasInsightSelectors": false, + "IsOrganizationTrail": false + }, + ], + { + data: {}, + err: { + "message": "Access Denied", + "code": "AccessDenied", + "region": null, + "time": "2020-08-13T17:17:02.086Z", + "requestId": "0000000000000000", + "extendedRequestId": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "statusCode": 404, + "retryable": false, + "retryDelay": 52.88593440570173 + }, + }, + [ + { + "Name": "not-delete-me-ueoeuaou-ms", + "CreationDate": "2020-06-30T14:29:53.000Z" + }, + ], + 'delete-me-ueoeuaou-ms' + ); + + eks.run(cache, {ignore_bucket_not_in_account: false}, callback); + }); + }) +}); \ No newline at end of file From 6dc8ddd19a19bef5c41eeb21710cf89c7ac72884 Mon Sep 17 00:00:00 2001 From: Matt Skillman Date: Fri, 14 Aug 2020 12:40:00 -0400 Subject: [PATCH 5/7] remainder of edits added to enable optional setting to ignore buckets not in account --- .../cloudtrailBucketAccessLogging.spec.js | 6 +- .../cloudtrail/cloudtrailBucketDelete.spec.js | 8 +- .../aws/cloudtrail/cloudtrailBucketPrivate.js | 58 +++- .../cloudtrailBucketPrivate.spec.js | 276 ++++++++++++++++++ 4 files changed, 337 insertions(+), 11 deletions(-) create mode 100644 plugins/aws/cloudtrail/cloudtrailBucketPrivate.spec.js diff --git a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.spec.js b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.spec.js index f7a56b3eb6..0a4094fb99 100644 --- a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.spec.js +++ b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.spec.js @@ -1,6 +1,6 @@ -var assert = require('assert'); -var expect = require('chai').expect; -var eks = require('./cloudtrailBucketAccessLogging'); +const assert = require('assert'); +const expect = require('chai').expect; +const eks = require('./cloudtrailBucketAccessLogging'); const createCache = (descTrailsData, getBuckLogData, listBuckData, buckName) => { let to_return = { cloudtrail: { diff --git a/plugins/aws/cloudtrail/cloudtrailBucketDelete.spec.js b/plugins/aws/cloudtrail/cloudtrailBucketDelete.spec.js index 6250803c38..8e6d2b5b05 100644 --- a/plugins/aws/cloudtrail/cloudtrailBucketDelete.spec.js +++ b/plugins/aws/cloudtrail/cloudtrailBucketDelete.spec.js @@ -1,6 +1,6 @@ -var assert = require('assert'); -var expect = require('chai').expect; -var eks = require('./cloudtrailBucketDelete'); +const assert = require('assert'); +const expect = require('chai').expect; +const eks = require('./cloudtrailBucketDelete'); const createCache = (descTrailsData, getBuckVerData, listBuckData, buckName) => { let to_return = { cloudtrail: { @@ -25,7 +25,7 @@ const createCache = (descTrailsData, getBuckVerData, listBuckData, buckName) => return to_return }; -describe('cloudtrailBucketAccessLogging', function () { +describe('cloudtrailBucketDelete', function () { describe('run', function () { it('should PASS if CloudTrail logging bucket has MFADelete enabled', function (done) { const callback = (err, results) => { diff --git a/plugins/aws/cloudtrail/cloudtrailBucketPrivate.js b/plugins/aws/cloudtrail/cloudtrailBucketPrivate.js index 8d153475e5..a1861f8b5c 100644 --- a/plugins/aws/cloudtrail/cloudtrailBucketPrivate.js +++ b/plugins/aws/cloudtrail/cloudtrailBucketPrivate.js @@ -1,6 +1,22 @@ var async = require('async'); var helpers = require('../../../helpers/aws'); +function bucketIsInAccount(allBuckets, targetBucketName) { + for (let i in allBuckets) { + let bucket = allBuckets[i]; + if (bucket.Name === targetBucketName) { + return true; // target bucket present in account + } + } + return false; // not present in account +} + +function bucketExists(err) { + return !(err && + err.code && + err.code === 'NoSuchBucket'); +} + module.exports = { title: 'CloudTrail Bucket Private', category: 'CloudTrail', @@ -8,9 +24,21 @@ module.exports = { more_info: 'CloudTrail buckets contain large amounts of sensitive account data and should only be accessible by logged in users.', recommended_action: 'Set the S3 bucket access policy for all CloudTrail buckets to only allow known users to access its files.', link: 'http://docs.aws.amazon.com/AmazonS3/latest/dev/example-bucket-policies.html', - apis: ['CloudTrail:describeTrails', 'S3:getBucketAcl'], + apis: ['CloudTrail:describeTrails', 'S3:getBucketAcl', 'S3:listBuckets'], + settings: { + ignore_bucket_not_in_account: { + name: 'Ignore CloudTrail Buckets Not in Account', + description: 'enable to ignore cloudtrail buckets that are not in the account', + regex: '^(true|false)$', // string true or boolean true to enable, string false or boolean false to disable + default: false + }, + }, run: function(cache, settings, callback) { + var config = { + ignore_bucket_not_in_account: settings.ignore_bucket_not_in_account || this.settings.ignore_bucket_not_in_account.default + }; + if (config.ignore_bucket_not_in_account === 'false') config.ignore_bucket_not_in_account = false; var results = []; var source = {}; var regions = helpers.regions(settings); @@ -40,15 +68,37 @@ module.exports = { var s3Region = helpers.defaultRegion(settings); + const listBuckets = helpers.addSource(cache, source, ['s3', 'listBuckets', s3Region]); + if (!listBuckets) return rcb(null, results, source); + if (listBuckets.err || !listBuckets.data) { + helpers.addResult(results, 3, `Unable to query for S3 buckets: ${helpers.addError(listBuckets)}`); + return rcb(null, results, source); + } + var getBucketAcl = helpers.addSource(cache, source, ['s3', 'getBucketAcl', s3Region, trail.S3BucketName]); if (!getBucketAcl || getBucketAcl.err || !getBucketAcl.data) { - helpers.addResult(results, 3, - 'Error querying for bucket policy for bucket: ' + trail.S3BucketName + ': ' + helpers.addError(getBucketAcl), + if (!bucketExists(getBucketAcl.err)) { + helpers.addResult(results, 2, + 'Bucket: ' + trail.S3BucketName + ' does not exist' , + region, 'arn:aws:s3:::' + trail.S3BucketName); + + return cb(); + } + else if (config.ignore_bucket_not_in_account && !bucketIsInAccount(listBuckets.data, trail.S3BucketName)) { + helpers.addResult(results, 0, + 'Bucket: ' + trail.S3BucketName + ' is not in account', region, 'arn:aws:s3:::' + trail.S3BucketName); - return cb(); + return cb(); + } else { + helpers.addResult(results, 3, + 'Error querying for bucket policy for bucket: ' + trail.S3BucketName + ': ' + helpers.addError(getBucketAcl), + region, 'arn:aws:s3:::' + trail.S3BucketName); + + return cb(); + } } var allowsAllUsersTypes = []; diff --git a/plugins/aws/cloudtrail/cloudtrailBucketPrivate.spec.js b/plugins/aws/cloudtrail/cloudtrailBucketPrivate.spec.js new file mode 100644 index 0000000000..a5aecb12ff --- /dev/null +++ b/plugins/aws/cloudtrail/cloudtrailBucketPrivate.spec.js @@ -0,0 +1,276 @@ +const assert = require('assert'); +const expect = require('chai').expect; +const eks = require('./cloudtrailBucketPrivate'); +const createCache = (descTrailsData, getBuckAclData, listBuckData, buckName) => { // todo + let to_return = { + cloudtrail: { + describeTrails: { + 'us-east-1': { + data: descTrailsData + } + }, + }, + s3: { + getBucketAcl: { + 'us-east-1': {} + }, + listBuckets: { + 'us-east-1': { + data: listBuckData + } + }, + } + }; + to_return.s3.getBucketAcl['us-east-1'][buckName] = getBuckAclData; + return to_return +}; + +describe('cloudtrailBucketPrivate', function () { + describe('run', function () { + it('should PASS if CloudTrail logging bucket is NOT publicly accessible', function (done) { + const callback = (err, results) => { + expect(results.length).to.equal(1); + expect(results[0].status).to.equal(0); + done(); + }; + + const cache = createCache( + [ + { + "Name": "delete-me-ms", + "S3BucketName": "delete-me-ueoeuaou-ms", + "IncludeGlobalServiceEvents": true, + "IsMultiRegionTrail": true, + "HomeRegion": "us-east-1", + "TrailARN": "arn:aws:cloudtrail:us-east-1:000000000000:trail/delete-me-ms", + "LogFileValidationEnabled": true, + "HasCustomEventSelectors": true, + "HasInsightSelectors": false, + "IsOrganizationTrail": false + }, + ], + { + data: { + "Owner": { + "DisplayName": "bar", + "ID": "7009a8971cd538e11f6b6606438875e7c86c5b672f46db45460ddcd087d36c33" + }, + "Grants": [ + { + "Grantee": { + "DisplayName": "foo", + "ID": "7009a8971cd538e11f6b6606438875e7c86c5b672f46db45460ddcd087d36c32" + }, + "Permission": "FULL_CONTROL" + } + ], + }, + }, + [ + { + "Name": "delete-me-ueoeuaou-ms", + "CreationDate": "2020-06-30T14:29:53.000Z" + }, + ], + 'delete-me-ueoeuaou-ms' + ); + + eks.run(cache, {}, callback); + }); + + it('should FAIL if CloudTrail logging bucket IS publicly accessible', function (done) { + const callback = (err, results) => { + expect(results.length).to.equal(1); + expect(results[0].status).to.equal(2); + done(); + }; + + const cache = createCache( + [ + { + "Name": "delete-me-ms", + "S3BucketName": "delete-me-ueoeuaou-ms", + "IncludeGlobalServiceEvents": true, + "IsMultiRegionTrail": true, + "HomeRegion": "us-east-1", + "TrailARN": "arn:aws:cloudtrail:us-east-1:000000000000:trail/delete-me-ms", + "LogFileValidationEnabled": true, + "HasCustomEventSelectors": true, + "HasInsightSelectors": false, + "IsOrganizationTrail": false + }, + ], + { + data: { + "Owner": { + "DisplayName": "bar", + "ID": "7009a8971cd538e11f6b6606438875e7c86c5b672f46db45460ddcd087d36c33" + }, + "Grants": [ + { + "Grantee": { + "Type": "Group", + "URI": "http://acs.amazonaws.com/groups/global/AllUsers" + }, + "Permission": "FULL_CONTROL" + } + ], + }, + }, + [ + { + "Name": "delete-me-ueoeuaou-ms", + "CreationDate": "2020-06-30T14:29:53.000Z" + }, + ], + 'delete-me-ueoeuaou-ms' + ); + + eks.run(cache, {}, callback); + }); + + it('should FAIL if CloudTrail logging bucket does not exist', function (done) { + const callback = (err, results) => { + expect(results.length).to.equal(1); + expect(results[0].status).to.equal(2); + done(); + }; + + const cache = createCache( + [ + { + "Name": "delete-me-ms", + "S3BucketName": "delete-me-ueoeuaou-ms", + "IncludeGlobalServiceEvents": true, + "IsMultiRegionTrail": true, + "HomeRegion": "us-east-1", + "TrailARN": "arn:aws:cloudtrail:us-east-1:000000000000:trail/delete-me-ms", + "LogFileValidationEnabled": true, + "HasCustomEventSelectors": true, + "HasInsightSelectors": false, + "IsOrganizationTrail": false + }, + ], + { + data: {}, + err: { + "message": "The specified bucket does not exist", + "code": "NoSuchBucket", + "region": null, + "time": "2020-08-13T17:17:02.086Z", + "requestId": "0000000000000000", + "extendedRequestId": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "statusCode": 404, + "retryable": false, + "retryDelay": 52.88593440570173 + }, + }, + [ + { + "Name": "delete-me-ueoeuaou-ms", + "CreationDate": "2020-06-30T14:29:53.000Z" + }, + ], + 'delete-me-ueoeuaou-ms' + ); + + eks.run(cache, {}, callback); + }); + + it('should PASS if CloudTrail logging bucket is in another account, with ignore_bucket_not_in_account enabled', function (done) { + const callback = (err, results) => { + expect(results.length).to.equal(1); + expect(results[0].status).to.equal(0); + done(); + }; + + const cache = createCache( + [ + { + "Name": "delete-me-ms", + "S3BucketName": "delete-me-ueoeuaou-ms", + "IncludeGlobalServiceEvents": true, + "IsMultiRegionTrail": true, + "HomeRegion": "us-east-1", + "TrailARN": "arn:aws:cloudtrail:us-east-1:000000000000:trail/delete-me-ms", + "LogFileValidationEnabled": true, + "HasCustomEventSelectors": true, + "HasInsightSelectors": false, + "IsOrganizationTrail": false + }, + ], + { + data: {}, + err: { + "message": "Access Denied", + "code": "AccessDenied", + "region": null, + "time": "2020-08-13T17:17:02.086Z", + "requestId": "0000000000000000", + "extendedRequestId": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "statusCode": 404, + "retryable": false, + "retryDelay": 52.88593440570173 + }, + }, + [ + { + "Name": "not-delete-me-ueoeuaou-ms", + "CreationDate": "2020-06-30T14:29:53.000Z" + }, + ], + 'delete-me-ueoeuaou-ms' + ); + + eks.run(cache, {ignore_bucket_not_in_account: true}, callback); + }); + + it('should UNKNOWN if CloudTrail logging bucket is in another account, with ignore_bucket_not_in_account disabled', function (done) { + const callback = (err, results) => { + expect(results.length).to.equal(1); + expect(results[0].status).to.equal(3); + done(); + }; + + const cache = createCache( + [ + { + "Name": "delete-me-ms", + "S3BucketName": "delete-me-ueoeuaou-ms", + "IncludeGlobalServiceEvents": true, + "IsMultiRegionTrail": true, + "HomeRegion": "us-east-1", + "TrailARN": "arn:aws:cloudtrail:us-east-1:000000000000:trail/delete-me-ms", + "LogFileValidationEnabled": true, + "HasCustomEventSelectors": true, + "HasInsightSelectors": false, + "IsOrganizationTrail": false + }, + ], + { + data: {}, + err: { + "message": "Access Denied", + "code": "AccessDenied", + "region": null, + "time": "2020-08-13T17:17:02.086Z", + "requestId": "0000000000000000", + "extendedRequestId": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "statusCode": 404, + "retryable": false, + "retryDelay": 52.88593440570173 + }, + }, + [ + { + "Name": "not-delete-me-ueoeuaou-ms", + "CreationDate": "2020-06-30T14:29:53.000Z" + }, + ], + 'delete-me-ueoeuaou-ms' + ); + + eks.run(cache, {ignore_bucket_not_in_account: false}, callback); + }); + }) +}); \ No newline at end of file From c156fe4ac16dae570ea240176efe9ad41b259725 Mon Sep 17 00:00:00 2001 From: Matt Skillman Date: Tue, 18 Aug 2020 13:58:34 -0400 Subject: [PATCH 6/7] bugfix of 'unable to return data' if bucket is not in account, correction of callbacks being called within files --- plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js | 6 +++--- plugins/aws/cloudtrail/cloudtrailBucketDelete.js | 6 +++--- plugins/aws/cloudtrail/cloudtrailBucketPrivate.js | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js index e789a4e957..2ecd422fa9 100644 --- a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js +++ b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js @@ -74,17 +74,17 @@ module.exports = { var s3Region = helpers.defaultRegion(settings); const listBuckets = helpers.addSource(cache, source, ['s3', 'listBuckets', s3Region]); - if (!listBuckets) return rcb(null, results, source); + if (!listBuckets) return cb(null, results, source); if (listBuckets.err || !listBuckets.data) { helpers.addResult(results, 3, `Unable to query for S3 buckets: ${helpers.addError(listBuckets)}`); - return rcb(null, results, source); + return cb(null, results, source); } var getBucketLogging = helpers.addSource(cache, source, ['s3', 'getBucketLogging', s3Region, trail.S3BucketName]); if (!getBucketLogging || getBucketLogging.err) { // data will be {} if logging is disabled. - if (!bucketExists(getBucketLogging.err)) { + if (getBucketLogging && !bucketExists(getBucketLogging.err)) { helpers.addResult(results, 2, 'Bucket: ' + trail.S3BucketName + ' does not exist' , region, 'arn:aws:s3:::' + trail.S3BucketName); diff --git a/plugins/aws/cloudtrail/cloudtrailBucketDelete.js b/plugins/aws/cloudtrail/cloudtrailBucketDelete.js index ce9908392c..30e844cdbe 100644 --- a/plugins/aws/cloudtrail/cloudtrailBucketDelete.js +++ b/plugins/aws/cloudtrail/cloudtrailBucketDelete.js @@ -75,17 +75,17 @@ module.exports = { var s3Region = helpers.defaultRegion(settings); const listBuckets = helpers.addSource(cache, source, ['s3', 'listBuckets', s3Region]); - if (!listBuckets) return rcb(null, results, source); + if (!listBuckets) return cb(null, results, source); if (listBuckets.err || !listBuckets.data) { helpers.addResult(results, 3, `Unable to query for S3 buckets: ${helpers.addError(listBuckets)}`); - return rcb(null, results, source); + return cb(null, results, source); } var getBucketVersioning = helpers.addSource(cache, source, ['s3', 'getBucketVersioning', s3Region, trail.S3BucketName]); if (!getBucketVersioning || getBucketVersioning.err || !getBucketVersioning.data) { // data is {} if disabled. this assumes other plugin checks to see if enabled. - if (!bucketExists(getBucketVersioning.err)) { + if (getBucketVersioning && !bucketExists(getBucketVersioning.err)) { helpers.addResult(results, 2, 'Bucket: ' + trail.S3BucketName + ' does not exist' , region, 'arn:aws:s3:::' + trail.S3BucketName); diff --git a/plugins/aws/cloudtrail/cloudtrailBucketPrivate.js b/plugins/aws/cloudtrail/cloudtrailBucketPrivate.js index a1861f8b5c..bd5f535919 100644 --- a/plugins/aws/cloudtrail/cloudtrailBucketPrivate.js +++ b/plugins/aws/cloudtrail/cloudtrailBucketPrivate.js @@ -69,17 +69,17 @@ module.exports = { var s3Region = helpers.defaultRegion(settings); const listBuckets = helpers.addSource(cache, source, ['s3', 'listBuckets', s3Region]); - if (!listBuckets) return rcb(null, results, source); + if (!listBuckets) return cb(null, results, source); if (listBuckets.err || !listBuckets.data) { helpers.addResult(results, 3, `Unable to query for S3 buckets: ${helpers.addError(listBuckets)}`); - return rcb(null, results, source); + return cb(null, results, source); } var getBucketAcl = helpers.addSource(cache, source, ['s3', 'getBucketAcl', s3Region, trail.S3BucketName]); if (!getBucketAcl || getBucketAcl.err || !getBucketAcl.data) { - if (!bucketExists(getBucketAcl.err)) { + if (getBucketAcl && !bucketExists(getBucketAcl.err)) { helpers.addResult(results, 2, 'Bucket: ' + trail.S3BucketName + ' does not exist' , region, 'arn:aws:s3:::' + trail.S3BucketName); From 54146259f7987924cf7850283381cf8d73ac9e8f Mon Sep 17 00:00:00 2001 From: jhaubold Date: Thu, 27 Aug 2020 13:08:15 -0400 Subject: [PATCH 7/7] cosmetic fix --- plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js | 6 +++--- plugins/aws/cloudtrail/cloudtrailBucketDelete.js | 6 +++--- plugins/aws/cloudtrail/cloudtrailBucketPrivate.js | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js index 2ecd422fa9..d28a7cdde7 100644 --- a/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js +++ b/plugins/aws/cloudtrail/cloudtrailBucketAccessLogging.js @@ -74,10 +74,10 @@ module.exports = { var s3Region = helpers.defaultRegion(settings); const listBuckets = helpers.addSource(cache, source, ['s3', 'listBuckets', s3Region]); - if (!listBuckets) return cb(null, results, source); + if (!listBuckets) return cb(); if (listBuckets.err || !listBuckets.data) { helpers.addResult(results, 3, `Unable to query for S3 buckets: ${helpers.addError(listBuckets)}`); - return cb(null, results, source); + return cb(); } var getBucketLogging = helpers.addSource(cache, source, @@ -126,4 +126,4 @@ module.exports = { callback(null, results, source); }); } -}; \ No newline at end of file +}; diff --git a/plugins/aws/cloudtrail/cloudtrailBucketDelete.js b/plugins/aws/cloudtrail/cloudtrailBucketDelete.js index 30e844cdbe..922568ff33 100644 --- a/plugins/aws/cloudtrail/cloudtrailBucketDelete.js +++ b/plugins/aws/cloudtrail/cloudtrailBucketDelete.js @@ -75,10 +75,10 @@ module.exports = { var s3Region = helpers.defaultRegion(settings); const listBuckets = helpers.addSource(cache, source, ['s3', 'listBuckets', s3Region]); - if (!listBuckets) return cb(null, results, source); + if (!listBuckets) return cb(); if (listBuckets.err || !listBuckets.data) { helpers.addResult(results, 3, `Unable to query for S3 buckets: ${helpers.addError(listBuckets)}`); - return cb(null, results, source); + return cb(); } var getBucketVersioning = helpers.addSource(cache, source, @@ -126,4 +126,4 @@ module.exports = { callback(null, results, source); }); } -}; \ No newline at end of file +}; diff --git a/plugins/aws/cloudtrail/cloudtrailBucketPrivate.js b/plugins/aws/cloudtrail/cloudtrailBucketPrivate.js index bd5f535919..0fd5e9d90c 100644 --- a/plugins/aws/cloudtrail/cloudtrailBucketPrivate.js +++ b/plugins/aws/cloudtrail/cloudtrailBucketPrivate.js @@ -69,10 +69,10 @@ module.exports = { var s3Region = helpers.defaultRegion(settings); const listBuckets = helpers.addSource(cache, source, ['s3', 'listBuckets', s3Region]); - if (!listBuckets) return cb(null, results, source); + if (!listBuckets) return cb(); if (listBuckets.err || !listBuckets.data) { helpers.addResult(results, 3, `Unable to query for S3 buckets: ${helpers.addError(listBuckets)}`); - return cb(null, results, source); + return cb(); } var getBucketAcl = helpers.addSource(cache, source, @@ -131,4 +131,4 @@ module.exports = { callback(null, results, source); }); } -}; \ No newline at end of file +};