Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions config.json
Original file line number Diff line number Diff line change
Expand Up @@ -142,5 +142,8 @@
},
"kmip": {
"providerName": "thales"
},
"integrityChecks": {
"objectPutRetention": true
}
}
34 changes: 34 additions & 0 deletions lib/Config.js
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,37 @@ function bucketNotifAssert(bucketNotifConfig) {
return bucketNotifConfig;
}

function parseIntegrityChecks(config) {
const integrityChecks = {
'bucketPutACL': true,
'bucketPutCors': true,
'bucketPutEncryption': true,
'bucketPutLifecycle': true,
'bucketPutNotification': true,
'bucketPutObjectLock': true,
'bucketPutPolicy': true,
'bucketPutReplication': true,
'bucketPutVersioning': true,
'bucketPutWebsite': true,
'multiObjectDelete': true,
'objectPutACL': true,
'objectPutLegalHold': true,
'objectPutTagging': true,
'objectPutRetention': true,
};

if (config && config.integrityChecks) {
for (const method in integrityChecks) {
if (method in config.integrityChecks) {
assert(typeof config.integrityChecks[method] == 'boolean', `bad config: ${method} not boolean`);
integrityChecks[method] = config.integrityChecks[method];
}
}
}

return integrityChecks;
}

/**
* Reads from a config file and returns the content as a config object
*/
Expand Down Expand Up @@ -1743,6 +1774,8 @@ class Config extends EventEmitter {

this.supportedLifecycleRules = parseSupportedLifecycleRules(config.supportedLifecycleRules);

this.integrityChecks = parseIntegrityChecks(config);

/**
* S3C-10336: PutObject max size of 5GB is new in 9.5.1
* Provides a way to bypass the new validation if it breaks customer workflows
Expand Down Expand Up @@ -2081,4 +2114,5 @@ module.exports = {
azureGetStorageAccountName,
azureGetLocationCredentials,
parseSupportedLifecycleRules,
parseIntegrityChecks,
};
11 changes: 10 additions & 1 deletion lib/api/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ const parseCopySource = require('./apiUtils/object/parseCopySource');
const { tagConditionKeyAuth } = require('./apiUtils/authorization/tagConditionKeys');
const { isRequesterASessionUser } = require('./apiUtils/authorization/permissionChecks');
const checkHttpHeadersSize = require('./apiUtils/object/checkHttpHeadersSize');
const { validateMethodChecksumNoChunking } = require('./apiUtils/integrity/validateChecksums');

const monitoringMap = policies.actionMaps.actionMonitoringMapS3;

Expand Down Expand Up @@ -242,8 +243,16 @@ const api = {
{ postLength });
return next(errors.InvalidRequest);
}

const buff = Buffer.concat(post, postLength);

const err = validateMethodChecksumNoChunking(request, buff, log);
if (err) {
return next(err);
}

// Convert array of post buffers into one string
request.post = Buffer.concat(post, postLength).toString();
request.post = buff.toString();
return next(null, userInfo, authorizationResults, streamingV4Params, infos);
});
return undefined;
Expand Down
82 changes: 82 additions & 0 deletions lib/api/apiUtils/integrity/validateChecksums.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
const crypto = require('crypto');
const { errors: ArsenalErrors } = require('arsenal');
const { config } = require('../../../Config');

const ChecksumError = Object.freeze({
MD5Mismatch: 'MD5Mismatch',
MissingChecksum: 'MissingChecksum',
});

/**
* validateChecksumsNoChunking - Validate the checksums of a request.
* @param {object} headers - http headers
* @param {Buffer} body - http request body
* @return {object} - error
*/
function validateChecksumsNoChunking(headers, body) {
if (headers && 'content-md5' in headers) {
const md5 = crypto.createHash('md5').update(body).digest('base64');
if (md5 !== headers['content-md5']) {
return { error: ChecksumError.MD5Mismatch, details: { calculated: md5, expected: headers['content-md5'] } };
}
Comment on lines +20 to +21
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently do nothing of this object, when we handle errors. Should we just return the error and add a log in this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the caller should do the logging.

In the next iteration I will add SHA256Mismatch, CRC32Mismatch, etc. I don't see why we would want to squash all the possible errors into BadDigest


return null;
}

return { error: ChecksumError.MissingChecksum, details: null };
}

function defaultValidationFunc(request, body, log) {
const err = validateChecksumsNoChunking(request.headers, body);
if (err && err.error !== ChecksumError.MissingChecksum) {
log.debug('failed checksum validation', { method: request.apiMethod }, err);
return ArsenalErrors.BadDigest;
}

return null;
}

const methodValidationFunc = Object.freeze({
'bucketPutACL': defaultValidationFunc,
'bucketPutCors': defaultValidationFunc,
'bucketPutEncryption': defaultValidationFunc,
'bucketPutLifecycle': defaultValidationFunc,
'bucketPutNotification': defaultValidationFunc,
'bucketPutObjectLock': defaultValidationFunc,
'bucketPutPolicy': defaultValidationFunc,
'bucketPutReplication': defaultValidationFunc,
'bucketPutVersioning': defaultValidationFunc,
'bucketPutWebsite': defaultValidationFunc,
// TODO: DeleteObjects requires a checksum. Should return an error if ChecksumError.MissingChecksum.
'multiObjectDelete': defaultValidationFunc,
'objectPutACL': defaultValidationFunc,
'objectPutLegalHold': defaultValidationFunc,
'objectPutTagging': defaultValidationFunc,
'objectPutRetention': defaultValidationFunc,
});

/**
* validateMethodChecksumsNoChunking - Validate the checksums of a request.
* @param {object} request - http request
* @param {Buffer} body - http request body
* @param {object} log - logger
* @return {object} - error
*/
function validateMethodChecksumNoChunking(request, body, log) {
if (config.integrityChecks[request.apiMethod]) {
const validationFunc = methodValidationFunc[request.apiMethod];
if (!validationFunc) {
return null;
}

return validationFunc(request, body, log);
}

return null;
}

module.exports = {
ChecksumError,
validateChecksumsNoChunking,
validateMethodChecksumNoChunking,
};
11 changes: 0 additions & 11 deletions lib/api/bucketPutCors.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
const crypto = require('crypto');
const async = require('async');
const { errors, errorInstances } = require('arsenal');

Expand Down Expand Up @@ -33,16 +32,6 @@ function bucketPutCors(authInfo, request, log, callback) {
return callback(errors.MissingRequestBodyError);
}

if (request.headers['content-md5']) {
const md5 = crypto.createHash('md5')
.update(request.post, 'utf8').digest('base64');
if (md5 !== request.headers['content-md5']) {
log.debug('bad md5 digest', { error: errors.BadDigest });
monitoring.promMetrics('PUT', bucketName, 400, 'putBucketCors');
return callback(errors.BadDigest);
}
}

if (parseInt(request.headers['content-length'], 10) > 65536) {
const errMsg = 'The CORS XML document is limited to 64 KB in size.';
log.debug(errMsg, { error: errors.MalformedXML });
Expand Down
1 change: 1 addition & 0 deletions lib/api/bucketPutReplication.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ function bucketPutReplication(authInfo, request, log, callback) {
requestType: request.apiMethods || 'bucketPutReplication',
request,
};

return waterfall([
// Validate the request XML and return the replication configuration.
next => getReplicationConfiguration(post, log, next),
Expand Down
12 changes: 0 additions & 12 deletions lib/api/multiObjectDelete.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
const crypto = require('crypto');

const async = require('async');
const { parseString } = require('xml2js');
const { auth, errors, versioning, s3middleware, policies } = require('arsenal');
Expand Down Expand Up @@ -475,16 +473,6 @@ function multiObjectDelete(authInfo, request, log, callback) {
return callback(errors.MissingRequestBodyError);
}

if (request.headers['content-md5']) {
const md5 = crypto.createHash('md5')
.update(request.post, 'utf8').digest('base64');
if (md5 !== request.headers['content-md5']) {
monitoring.promMetrics('DELETE', request.bucketName, 400,
'multiObjectDelete');
return callback(errors.BadDigest);
}
}

const inPlayInternal = [];
const bucketName = request.bucketName;
const canonicalID = authInfo.getCanonicalID();
Expand Down
1 change: 0 additions & 1 deletion lib/api/objectPutTagging.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ function objectPutTagging(authInfo, request, log, callback) {
log.debug('processing request', { method: 'objectPutTagging' });

const { bucketName, objectKey } = request;

const decodedVidResult = decodeVersionId(request.query);
if (decodedVidResult instanceof Error) {
log.trace('invalid versionId query', {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@zenko/cloudserver",
"version": "9.0.26",
"version": "9.0.27",
"description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol",
"main": "index.js",
"engines": {
Expand Down
37 changes: 37 additions & 0 deletions tests/unit/Config.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const {
azureGetLocationCredentials,
locationConstraintAssert,
parseSupportedLifecycleRules,
parseIntegrityChecks,
ConfigObject,
} = require('../../lib/Config');

Expand Down Expand Up @@ -885,4 +886,40 @@ describe('Config', () => {
assert.strictEqual(config.instanceId.length, 6);
});
});

describe('parse integrity checks', () => {
it('should replace default values with new values', () => {
const newConfig = {
integrityChecks: {
'bucketPutACL': false,
'bucketPutCors': false,
'bucketPutEncryption': false,
'bucketPutLifecycle': false,
'bucketPutNotification': false,
'bucketPutObjectLock': false,
'bucketPutPolicy': false,
'bucketPutReplication': false,
'bucketPutVersioning': false,
'bucketPutWebsite': false,
'multiObjectDelete': false,
'objectPutACL': false,
'objectPutLegalHold': false,
'objectPutTagging': false,
'objectPutRetention': false,
},
};

const result = parseIntegrityChecks(newConfig);
for (const method in result) {
assert(result[method] == false, method);
}
});

it('default method value is true', () => {
const result = parseIntegrityChecks(null);
for (const method in result) {
assert(result[method] == true, method);
}
});
});
});
Loading
Loading