added 20kb limit for put bucket policy

Issue : CLDSRV-700

Author: SylvainSenechal

config.json

@@ -142,5 +142,9 @@
     },
     "kmip": {
         "providerName": "thales"
+    },
+    "apiBodySizeLimits": {
+        "multiObjectDelete": 2097152,
+        "bucketPutPolicy": 20480
     }
 }

constants.js

@@ -96,11 +96,15 @@ const constants = {
     oneMegaBytes: 1024 * 1024,
     halfMegaBytes: 512 * 1024,
 
-    // Some apis may need a custom body length limit :
-    apisLengthLimits: {
+    // Some apis may need a custom body length limit
+    // Caution : Users will be able to override these values in config.json
+    defaultApiBodySizeLimits: {
         // Multi Objects Delete request can be large : up to 1000 keys of 1024 bytes is
         // already 1mb, with the other fields it could reach 2mb
         'multiObjectDelete': 2 * 1024 * 1024,
+        // AWS sets the maximum size for bucket policies to 20 KB
+        // https://docs.aws.amazon.com/AmazonS3/latest/userguide/add-bucket-policy.html
+        'bucketPutPolicy': 20 * 1024,
     },
 
     // hex digest of sha256 hash of empty string:
@@ -266,5 +270,4 @@ const constants = {
     onlyOwnerAllowed: ['bucketDeletePolicy', 'bucketGetPolicy', 'bucketPutPolicy'],
 };
 
-
 module.exports = constants;

lib/Config.js

@@ -1737,6 +1737,25 @@ class Config extends EventEmitter {
         }
 
         this.supportedLifecycleRules = parseSupportedLifecycleRules(config.supportedLifecycleRules);
+
+        this.apiBodySizeLimits = { ...constants.defaultApiBodySizeLimits };
+        if (config.apiBodySizeLimits) {
+            assert(typeof config.apiBodySizeLimits === 'object' &&
+                !Array.isArray(config.apiBodySizeLimits),
+                'bad config: apiBodySizeLimits must be an object');
+
+            for (const [apiKey, limit] of Object.entries(config.apiBodySizeLimits)) {
+                // Only allow modifications of predefined APIs from constants
+                assert(Object.hasOwn(constants.defaultApiBodySizeLimits, apiKey),
+                    `bad config: apiBodySizeLimits for "${apiKey}" cannot be configured. ` +
+                    `Valid APIs are: ${Object.keys(constants.defaultApiBodySizeLimits).join(', ')}`);
+                
+                assert(Number.isInteger(limit) && limit > 0,
+                    `bad config: apiBodySizeLimits for "${apiKey}" must be a positive integer`);
+                this.apiBodySizeLimits[apiKey] = limit;
+            }
+        }
+
         return config;
     }
 

lib/api/api.js

@@ -75,6 +75,7 @@ const { tagConditionKeyAuth } = require('./apiUtils/authorization/tagConditionKe
 const { isRequesterASessionUser } = require('./apiUtils/authorization/permissionChecks');
 const checkHttpHeadersSize = require('./apiUtils/object/checkHttpHeadersSize');
 const constants = require('../../constants');
+const { config } = require('../Config.js');
 
 const monitoringMap = policies.actionMaps.actionMonitoringMapS3;
 
@@ -219,15 +220,15 @@ const api = {
                 // issue 100 Continue to the client
                 writeContinue(request, response);
 
-                const defaultMaxPostLength = request.method === 'POST' ?
+                const defaultMaxBodyLength = request.method === 'POST' ?
                       constants.oneMegaBytes : constants.halfMegaBytes;
-                const MAX_POST_LENGTH = constants.apisLengthLimits[apiMethod] || defaultMaxPostLength;
+                const MAX_BODY_LENGTH = config.apiBodySizeLimits[apiMethod] || defaultMaxBodyLength;
                 const post = [];
-                let postLength = 0;
+                let bodyLength = 0;
                 request.on('data', chunk => {
-                    postLength += chunk.length;
+                    bodyLength += chunk.length;
                     // Sanity check on post length
-                    if (postLength <= MAX_POST_LENGTH) {
+                    if (bodyLength <= MAX_BODY_LENGTH) {
                         post.push(chunk);
                     }
                 });
@@ -240,13 +241,13 @@ const api = {
                 });
 
                 request.on('end', () => {
-                    if (postLength > MAX_POST_LENGTH) {
+                    if (bodyLength > MAX_BODY_LENGTH) {
                         log.error('body length is too long for request type',
-                                  { postLength });
+                                  { bodyLength });
                         return next(errors.InvalidRequest);
                     }
                     // Convert array of post buffers into one string
-                    request.post = Buffer.concat(post, postLength).toString();
+                    request.post = Buffer.concat(post, bodyLength).toString();
                     return next(null, userInfo, authorizationResults, streamingV4Params, infos);
                 });
                 return undefined;

tests/unit/Config.js

@@ -13,6 +13,7 @@ const {
 const {
     LOCATION_NAME_DMF,
 } = require('../constants');
+const constants = require('../../constants');
 
 const { ValidLifecycleRules: supportedLifecycleRules } = require('arsenal').models;
 
@@ -889,4 +890,47 @@ describe('Config', () => {
             assert.strictEqual(config.instanceId.length, 6);
         });
     });
+
+    describe('apisLengthLimits configuration', () => {
+        let sandbox;
+        let readFileStub;
+
+        beforeEach(() => {
+            sandbox = sinon.createSandbox();
+            readFileStub = sandbox.stub(fs, 'readFileSync');
+            readFileStub.callThrough();
+        });
+
+        afterEach(() => {
+            sandbox.restore();
+        });
+
+        it('should use default API and overwrite when config is provided', () => {
+            const multiObjectDeleteSize = 42;
+            const modifiedConfig = {
+                ...defaultConfig,
+                apiBodySizeLimits: { 'multiObjectDelete': multiObjectDeleteSize },
+            };
+            readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(modifiedConfig));
+            const config = new ConfigObject();
+
+            assert.deepStrictEqual(config.apiBodySizeLimits, {
+                'multiObjectDelete': multiObjectDeleteSize, // Configured: overwrites default
+                'bucketPutPolicy': constants.defaultApiBodySizeLimits['bucketPutPolicy'], // Not configured: default
+            });
+        });
+
+        it('should fail if a user tries to modify a non-existent API', () => {
+            const modifiedConfig = {
+                ...defaultConfig,
+                apiBodySizeLimits: { 'anApiNotSetInConstants.js': 42 },
+            };
+            readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(modifiedConfig));
+
+            assert.throws(
+                () => new ConfigObject(),
+                /bad config: apiBodySizeLimits for "anApiNotSetInConstants.js" cannot be configured/
+            );
+        });
+    });
 });