Skip to content

Commit 189e04e

Browse files
added 20kb limit for put bucket policy
Issue : CLDSRV-700
1 parent 4b53e38 commit 189e04e

File tree

5 files changed

+76
-4
lines changed

5 files changed

+76
-4
lines changed

config.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,5 +142,9 @@
142142
},
143143
"kmip": {
144144
"providerName": "thales"
145+
},
146+
"apiBodySizeLimits": {
147+
"multiObjectDelete": 2097152,
148+
"bucketPutPolicy": 20480
145149
}
146150
}

constants.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,15 @@ const constants = {
9696
oneMegaBytes: 1024 * 1024,
9797
halfMegaBytes: 512 * 1024,
9898

99-
// Some apis may need a custom body length limit :
100-
apisLengthLimits: {
99+
// Some apis may need a custom body length limit
100+
// Caution : Users will be able to override these values in config.json
101+
defaultApiBodySizeLimits: {
101102
// Multi Objects Delete request can be large : up to 1000 keys of 1024 bytes is
102103
// already 1mb, with the other fields it could reach 2mb
103104
'multiObjectDelete': 2 * 1024 * 1024,
105+
// AWS sets the maximum size for bucket policies to 20 KB
106+
// https://docs.aws.amazon.com/AmazonS3/latest/userguide/add-bucket-policy.html
107+
'bucketPutPolicy': 20 * 1024,
104108
},
105109

106110
// hex digest of sha256 hash of empty string:
@@ -266,5 +270,4 @@ const constants = {
266270
onlyOwnerAllowed: ['bucketDeletePolicy', 'bucketGetPolicy', 'bucketPutPolicy'],
267271
};
268272

269-
270273
module.exports = constants;

lib/Config.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1737,6 +1737,26 @@ class Config extends EventEmitter {
17371737
}
17381738

17391739
this.supportedLifecycleRules = parseSupportedLifecycleRules(config.supportedLifecycleRules);
1740+
1741+
this.apiBodySizeLimits = { ...constants.defaultApiBodySizeLimits };
1742+
if (config.apiBodySizeLimits) {
1743+
assert(typeof config.apiBodySizeLimits === 'object' &&
1744+
config.apiBodySizeLimits !== null &&
1745+
!Array.isArray(config.apiBodySizeLimits),
1746+
'bad config: apiBodySizeLimits must be an object');
1747+
1748+
for (const [apiKey, limit] of Object.entries(config.apiBodySizeLimits)) {
1749+
// Only allow modifications of predefined APIs from constants
1750+
assert(Object.hasOwn(constants.defaultApiBodySizeLimits, apiKey),
1751+
`bad config: apiBodySizeLimits for "${apiKey}" cannot be configured. ` +
1752+
`Valid APIs are: ${Object.keys(constants.defaultApiBodySizeLimits).join(', ')}`);
1753+
1754+
assert(Number.isInteger(limit) && limit > 0,
1755+
`bad config: apiBodySizeLimits for "${apiKey}" must be a positive integer`);
1756+
this.apiBodySizeLimits[apiKey] = limit;
1757+
}
1758+
}
1759+
17401760
return config;
17411761
}
17421762

lib/api/api.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ const { tagConditionKeyAuth } = require('./apiUtils/authorization/tagConditionKe
7575
const { isRequesterASessionUser } = require('./apiUtils/authorization/permissionChecks');
7676
const checkHttpHeadersSize = require('./apiUtils/object/checkHttpHeadersSize');
7777
const constants = require('../../constants');
78+
const { config } = require('../Config.js');
7879

7980
const monitoringMap = policies.actionMaps.actionMonitoringMapS3;
8081

@@ -221,7 +222,7 @@ const api = {
221222

222223
const defaultMaxPostLength = request.method === 'POST' ?
223224
constants.oneMegaBytes : constants.halfMegaBytes;
224-
const MAX_POST_LENGTH = constants.apisLengthLimits[apiMethod] || defaultMaxPostLength;
225+
const MAX_POST_LENGTH = config.apiBodySizeLimits[apiMethod] || defaultMaxPostLength;
225226
const post = [];
226227
let postLength = 0;
227228
request.on('data', chunk => {

tests/unit/Config.js

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const {
1313
const {
1414
LOCATION_NAME_DMF,
1515
} = require('../constants');
16+
const constants = require('../../constants');
1617

1718
const { ValidLifecycleRules: supportedLifecycleRules } = require('arsenal').models;
1819

@@ -889,4 +890,47 @@ describe('Config', () => {
889890
assert.strictEqual(config.instanceId.length, 6);
890891
});
891892
});
893+
894+
describe('apisLengthLimits configuration', () => {
895+
let sandbox;
896+
let readFileStub;
897+
898+
beforeEach(() => {
899+
sandbox = sinon.createSandbox();
900+
readFileStub = sandbox.stub(fs, 'readFileSync');
901+
readFileStub.callThrough();
902+
});
903+
904+
afterEach(() => {
905+
sandbox.restore();
906+
});
907+
908+
it('should use default API and overwrite when config is provided', () => {
909+
const multiObjectDeleteSize = 42;
910+
const modifiedConfig = {
911+
...defaultConfig,
912+
apiBodySizeLimits: { 'multiObjectDelete': multiObjectDeleteSize },
913+
};
914+
readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(modifiedConfig));
915+
const config = new ConfigObject();
916+
917+
assert.deepStrictEqual(config.apiBodySizeLimits, {
918+
'multiObjectDelete': multiObjectDeleteSize, // Configured: overwrites default
919+
'bucketPutPolicy': constants.defaultApiBodySizeLimits['bucketPutPolicy'], // Not configured: default
920+
});
921+
});
922+
923+
it('should fail if a user tries to modify a non-existent API', () => {
924+
const modifiedConfig = {
925+
...defaultConfig,
926+
apiBodySizeLimits: { 'anApiNotSetInConstants.js': 42 },
927+
};
928+
readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(modifiedConfig));
929+
930+
assert.throws(
931+
() => new ConfigObject(),
932+
/bad config: apiBodySizeLimits for "anApiNotSetInConstants.js" cannot be configured/
933+
);
934+
});
935+
});
892936
});

0 commit comments

Comments
 (0)