Skip to content

Commit 1d1c6c1

Browse files
committed
Merge branch 'feature/CLDSRV-744-check-content-md5-2' into q/9.0
2 parents 13e3c36 + befeb4e commit 1d1c6c1

File tree

17 files changed

+543
-215
lines changed

17 files changed

+543
-215
lines changed

config.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,5 +142,8 @@
142142
},
143143
"kmip": {
144144
"providerName": "thales"
145+
},
146+
"integrityChecks": {
147+
"objectPutRetention": true
145148
}
146149
}

lib/Config.js

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,37 @@ function bucketNotifAssert(bucketNotifConfig) {
533533
return bucketNotifConfig;
534534
}
535535

536+
function parseIntegrityChecks(config) {
537+
const integrityChecks = {
538+
'bucketPutACL': true,
539+
'bucketPutCors': true,
540+
'bucketPutEncryption': true,
541+
'bucketPutLifecycle': true,
542+
'bucketPutNotification': true,
543+
'bucketPutObjectLock': true,
544+
'bucketPutPolicy': true,
545+
'bucketPutReplication': true,
546+
'bucketPutVersioning': true,
547+
'bucketPutWebsite': true,
548+
'multiObjectDelete': true,
549+
'objectPutACL': true,
550+
'objectPutLegalHold': true,
551+
'objectPutTagging': true,
552+
'objectPutRetention': true,
553+
};
554+
555+
if (config && config.integrityChecks) {
556+
for (const method in integrityChecks) {
557+
if (method in config.integrityChecks) {
558+
assert(typeof config.integrityChecks[method] == 'boolean', `bad config: ${method} not boolean`);
559+
integrityChecks[method] = config.integrityChecks[method];
560+
}
561+
}
562+
}
563+
564+
return integrityChecks;
565+
}
566+
536567
/**
537568
* Reads from a config file and returns the content as a config object
538569
*/
@@ -1743,6 +1774,8 @@ class Config extends EventEmitter {
17431774

17441775
this.supportedLifecycleRules = parseSupportedLifecycleRules(config.supportedLifecycleRules);
17451776

1777+
this.integrityChecks = parseIntegrityChecks(config);
1778+
17461779
/**
17471780
* S3C-10336: PutObject max size of 5GB is new in 9.5.1
17481781
* Provides a way to bypass the new validation if it breaks customer workflows
@@ -2081,4 +2114,5 @@ module.exports = {
20812114
azureGetStorageAccountName,
20822115
azureGetLocationCredentials,
20832116
parseSupportedLifecycleRules,
2117+
parseIntegrityChecks,
20842118
};

lib/api/api.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ const parseCopySource = require('./apiUtils/object/parseCopySource');
7474
const { tagConditionKeyAuth } = require('./apiUtils/authorization/tagConditionKeys');
7575
const { isRequesterASessionUser } = require('./apiUtils/authorization/permissionChecks');
7676
const checkHttpHeadersSize = require('./apiUtils/object/checkHttpHeadersSize');
77+
const { validateMethodChecksumNoChunking } = require('./apiUtils/integrity/validateChecksums');
7778

7879
const monitoringMap = policies.actionMaps.actionMonitoringMapS3;
7980

@@ -242,8 +243,16 @@ const api = {
242243
{ postLength });
243244
return next(errors.InvalidRequest);
244245
}
246+
247+
const buff = Buffer.concat(post, postLength);
248+
249+
const err = validateMethodChecksumNoChunking(request, buff, log);
250+
if (err) {
251+
return next(err);
252+
}
253+
245254
// Convert array of post buffers into one string
246-
request.post = Buffer.concat(post, postLength).toString();
255+
request.post = buff.toString();
247256
return next(null, userInfo, authorizationResults, streamingV4Params, infos);
248257
});
249258
return undefined;
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
const crypto = require('crypto');
2+
const { errors: ArsenalErrors } = require('arsenal');
3+
const { config } = require('../../../Config');
4+
5+
const ChecksumError = Object.freeze({
6+
MD5Mismatch: 'MD5Mismatch',
7+
MissingChecksum: 'MissingChecksum',
8+
});
9+
10+
/**
11+
* validateChecksumsNoChunking - Validate the checksums of a request.
12+
* @param {object} headers - http headers
13+
* @param {Buffer} body - http request body
14+
* @return {object} - error
15+
*/
16+
function validateChecksumsNoChunking(headers, body) {
17+
if (headers && 'content-md5' in headers) {
18+
const md5 = crypto.createHash('md5').update(body).digest('base64');
19+
if (md5 !== headers['content-md5']) {
20+
return { error: ChecksumError.MD5Mismatch, details: { calculated: md5, expected: headers['content-md5'] } };
21+
}
22+
23+
return null;
24+
}
25+
26+
return { error: ChecksumError.MissingChecksum, details: null };
27+
}
28+
29+
function defaultValidationFunc(request, body, log) {
30+
const err = validateChecksumsNoChunking(request.headers, body);
31+
if (err && err.error !== ChecksumError.MissingChecksum) {
32+
log.debug('failed checksum validation', { method: request.apiMethod }, err);
33+
return ArsenalErrors.BadDigest;
34+
}
35+
36+
return null;
37+
}
38+
39+
const methodValidationFunc = Object.freeze({
40+
'bucketPutACL': defaultValidationFunc,
41+
'bucketPutCors': defaultValidationFunc,
42+
'bucketPutEncryption': defaultValidationFunc,
43+
'bucketPutLifecycle': defaultValidationFunc,
44+
'bucketPutNotification': defaultValidationFunc,
45+
'bucketPutObjectLock': defaultValidationFunc,
46+
'bucketPutPolicy': defaultValidationFunc,
47+
'bucketPutReplication': defaultValidationFunc,
48+
'bucketPutVersioning': defaultValidationFunc,
49+
'bucketPutWebsite': defaultValidationFunc,
50+
// TODO: DeleteObjects requires a checksum. Should return an error if ChecksumError.MissingChecksum.
51+
'multiObjectDelete': defaultValidationFunc,
52+
'objectPutACL': defaultValidationFunc,
53+
'objectPutLegalHold': defaultValidationFunc,
54+
'objectPutTagging': defaultValidationFunc,
55+
'objectPutRetention': defaultValidationFunc,
56+
});
57+
58+
/**
59+
* validateMethodChecksumsNoChunking - Validate the checksums of a request.
60+
* @param {object} request - http request
61+
* @param {Buffer} body - http request body
62+
* @param {object} log - logger
63+
* @return {object} - error
64+
*/
65+
function validateMethodChecksumNoChunking(request, body, log) {
66+
if (config.integrityChecks[request.apiMethod]) {
67+
const validationFunc = methodValidationFunc[request.apiMethod];
68+
if (!validationFunc) {
69+
return null;
70+
}
71+
72+
return validationFunc(request, body, log);
73+
}
74+
75+
return null;
76+
}
77+
78+
module.exports = {
79+
ChecksumError,
80+
validateChecksumsNoChunking,
81+
validateMethodChecksumNoChunking,
82+
};

lib/api/bucketPutCors.js

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
const crypto = require('crypto');
21
const async = require('async');
32
const { errors, errorInstances } = require('arsenal');
43

@@ -33,16 +32,6 @@ function bucketPutCors(authInfo, request, log, callback) {
3332
return callback(errors.MissingRequestBodyError);
3433
}
3534

36-
if (request.headers['content-md5']) {
37-
const md5 = crypto.createHash('md5')
38-
.update(request.post, 'utf8').digest('base64');
39-
if (md5 !== request.headers['content-md5']) {
40-
log.debug('bad md5 digest', { error: errors.BadDigest });
41-
monitoring.promMetrics('PUT', bucketName, 400, 'putBucketCors');
42-
return callback(errors.BadDigest);
43-
}
44-
}
45-
4635
if (parseInt(request.headers['content-length'], 10) > 65536) {
4736
const errMsg = 'The CORS XML document is limited to 64 KB in size.';
4837
log.debug(errMsg, { error: errors.MalformedXML });

lib/api/bucketPutReplication.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ function bucketPutReplication(authInfo, request, log, callback) {
3333
requestType: request.apiMethods || 'bucketPutReplication',
3434
request,
3535
};
36+
3637
return waterfall([
3738
// Validate the request XML and return the replication configuration.
3839
next => getReplicationConfiguration(post, log, next),

lib/api/multiObjectDelete.js

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
const crypto = require('crypto');
2-
31
const async = require('async');
42
const { parseString } = require('xml2js');
53
const { auth, errors, versioning, s3middleware, policies } = require('arsenal');
@@ -475,16 +473,6 @@ function multiObjectDelete(authInfo, request, log, callback) {
475473
return callback(errors.MissingRequestBodyError);
476474
}
477475

478-
if (request.headers['content-md5']) {
479-
const md5 = crypto.createHash('md5')
480-
.update(request.post, 'utf8').digest('base64');
481-
if (md5 !== request.headers['content-md5']) {
482-
monitoring.promMetrics('DELETE', request.bucketName, 400,
483-
'multiObjectDelete');
484-
return callback(errors.BadDigest);
485-
}
486-
}
487-
488476
const inPlayInternal = [];
489477
const bucketName = request.bucketName;
490478
const canonicalID = authInfo.getCanonicalID();

lib/api/objectPutTagging.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ function objectPutTagging(authInfo, request, log, callback) {
2727
log.debug('processing request', { method: 'objectPutTagging' });
2828

2929
const { bucketName, objectKey } = request;
30-
3130
const decodedVidResult = decodeVersionId(request.query);
3231
if (decodedVidResult instanceof Error) {
3332
log.trace('invalid versionId query', {

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@zenko/cloudserver",
3-
"version": "9.0.26",
3+
"version": "9.0.27",
44
"description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol",
55
"main": "index.js",
66
"engines": {

tests/unit/Config.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const {
77
azureGetLocationCredentials,
88
locationConstraintAssert,
99
parseSupportedLifecycleRules,
10+
parseIntegrityChecks,
1011
ConfigObject,
1112
} = require('../../lib/Config');
1213

@@ -885,4 +886,40 @@ describe('Config', () => {
885886
assert.strictEqual(config.instanceId.length, 6);
886887
});
887888
});
889+
890+
describe('parse integrity checks', () => {
891+
it('should replace default values with new values', () => {
892+
const newConfig = {
893+
integrityChecks: {
894+
'bucketPutACL': false,
895+
'bucketPutCors': false,
896+
'bucketPutEncryption': false,
897+
'bucketPutLifecycle': false,
898+
'bucketPutNotification': false,
899+
'bucketPutObjectLock': false,
900+
'bucketPutPolicy': false,
901+
'bucketPutReplication': false,
902+
'bucketPutVersioning': false,
903+
'bucketPutWebsite': false,
904+
'multiObjectDelete': false,
905+
'objectPutACL': false,
906+
'objectPutLegalHold': false,
907+
'objectPutTagging': false,
908+
'objectPutRetention': false,
909+
},
910+
};
911+
912+
const result = parseIntegrityChecks(newConfig);
913+
for (const method in result) {
914+
assert(result[method] == false, method);
915+
}
916+
});
917+
918+
it('default method value is true', () => {
919+
const result = parseIntegrityChecks(null);
920+
for (const method in result) {
921+
assert(result[method] == true, method);
922+
}
923+
});
924+
});
888925
});

0 commit comments

Comments
 (0)