-
Notifications
You must be signed in to change notification settings - Fork 254
CLDSRV-848: add missing checksum algorithms #6081
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
70121db
7bf172c
6964dba
07dc7ea
143423b
bf5fa48
8091c9a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,75 +1,249 @@ | ||
| const crypto = require('crypto'); | ||
| const { Crc32 } = require('@aws-crypto/crc32'); | ||
| const { Crc32c } = require('@aws-crypto/crc32c'); | ||
| const { CrtCrc64Nvme } = require('@aws-sdk/crc64-nvme-crt'); | ||
| const { errors: ArsenalErrors } = require('arsenal'); | ||
| const { config } = require('../../../Config'); | ||
|
|
||
| const checksumedMethods = Object.freeze({ | ||
| 'completeMultipartUpload': true, | ||
| 'multiObjectDelete': true, | ||
| 'bucketPutACL': true, | ||
| 'bucketPutCors': true, | ||
| 'bucketPutEncryption': true, | ||
| 'bucketPutLifecycle': true, | ||
| 'bucketPutLogging': true, | ||
| 'bucketPutNotification': true, | ||
| 'bucketPutPolicy': true, | ||
| 'bucketPutReplication': true, | ||
| 'bucketPutTagging': true, | ||
| 'bucketPutVersioning': true, | ||
| 'bucketPutWebsite': true, | ||
| 'objectPutACL': true, | ||
| 'objectPutLegalHold': true, | ||
| 'bucketPutObjectLock': true, // PutObjectLockConfiguration | ||
| 'objectPutRetention': true, | ||
| 'objectPutTagging': true, | ||
| 'objectRestore': true, | ||
| }); | ||
|
|
||
| const ChecksumError = Object.freeze({ | ||
| MD5Mismatch: 'MD5Mismatch', | ||
| MD5Invalid: 'MD5Invalid', | ||
| XAmzMismatch: 'XAmzMismatch', | ||
| MissingChecksum: 'MissingChecksum', | ||
| AlgoNotSupported: 'AlgoNotSupported', | ||
| AlgoNotSupportedSDK: 'AlgoNotSupportedSDK', | ||
| MultipleChecksumTypes: 'MultipleChecksumTypes', | ||
| MissingCorresponding: 'MissingCorresponding', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: rename to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer
|
||
| MalformedChecksum: 'MalformedChecksum', | ||
| }); | ||
|
|
||
| const base64Regex = /^[A-Za-z0-9+/]*={0,2}$/; | ||
|
|
||
| function uint32ToBase64(num) { | ||
| const buf = Buffer.alloc(4); | ||
| buf.writeUInt32BE(num, 0); | ||
| return buf.toString('base64'); | ||
| } | ||
|
|
||
| const algorithms = Object.freeze({ | ||
| crc64nvme: { | ||
| digest: async data => { | ||
| const input = Buffer.isBuffer(data) ? data : Buffer.from(data); | ||
| const crc = new CrtCrc64Nvme(); | ||
| crc.update(input); | ||
| const result = await crc.digest(); | ||
| return Buffer.from(result).toString('base64'); | ||
| }, | ||
| isValidDigest: expected => typeof expected === 'string' && expected.length === 12 && base64Regex.test(expected), | ||
| }, | ||
| crc32: { | ||
| digest: data => { | ||
| const input = Buffer.isBuffer(data) ? data : Buffer.from(data); | ||
| return uint32ToBase64(new Crc32().update(input).digest() >>> 0); // >>> 0 coerce number to uint32 | ||
| }, | ||
| isValidDigest: expected => typeof expected === 'string' && expected.length === 8 && base64Regex.test(expected), | ||
| }, | ||
| crc32c: { | ||
| digest: data => { | ||
| const input = Buffer.isBuffer(data) ? data : Buffer.from(data); | ||
| return uint32ToBase64(new Crc32c().update(input).digest() >>> 0); // >>> 0 coerce number to uint32 | ||
| }, | ||
| isValidDigest: expected => typeof expected === 'string' && expected.length === 8 && base64Regex.test(expected), | ||
| }, | ||
| sha1: { | ||
| digest: data => { | ||
| const input = Buffer.isBuffer(data) ? data : Buffer.from(data); | ||
| return crypto.createHash('sha1').update(input).digest('base64'); | ||
| }, | ||
| isValidDigest: expected => typeof expected === 'string' && expected.length === 28 && base64Regex.test(expected), | ||
| }, | ||
| sha256: { | ||
| digest: data => { | ||
| const input = Buffer.isBuffer(data) ? data : Buffer.from(data); | ||
| return crypto.createHash('sha256').update(input).digest('base64'); | ||
| }, | ||
| isValidDigest: expected => typeof expected === 'string' && expected.length === 44 && base64Regex.test(expected), | ||
| } | ||
| }); | ||
|
|
||
| async function validateXAmzChecksums(headers, body) { | ||
| const checksumHeaders = Object.keys(headers).filter(header => header.startsWith('x-amz-checksum-')); | ||
| const xAmzChecksumCnt = checksumHeaders.length; | ||
| if (xAmzChecksumCnt > 1) { | ||
| return { error: ChecksumError.MultipleChecksumTypes, details: { algorithms: checksumHeaders } }; | ||
| } | ||
|
|
||
| if (xAmzChecksumCnt === 0 && 'x-amz-sdk-checksum-algorithm' in headers) { | ||
| return { | ||
| error: ChecksumError.MissingCorresponding, | ||
| details: { expected: headers['x-amz-sdk-checksum-algorithm'] } | ||
| }; | ||
| } else if (xAmzChecksumCnt === 0) { | ||
| return { error: ChecksumError.MissingChecksum, details: null }; | ||
| } | ||
|
|
||
| // No x-amz-sdk-checksum-algorithm we expect one x-amz-checksum-[crc64nvme, crc32, crc32C, sha1, sha256]. | ||
| const algo = checksumHeaders[0].slice('x-amz-checksum-'.length); | ||
| if (!(algo in algorithms)) { | ||
| return { error: ChecksumError.AlgoNotSupported, details: { algorithm: algo } };; | ||
| } | ||
|
|
||
| const expected = headers[`x-amz-checksum-${algo}`]; | ||
| if (!algorithms[algo].isValidDigest(expected)) { | ||
| return { error: ChecksumError.MalformedChecksum, details: { algorithm: algo, expected } }; | ||
| } | ||
|
|
||
| const calculated = await algorithms[algo].digest(body); | ||
| if (expected !== calculated) { | ||
| return { error: ChecksumError.XAmzMismatch, details: { algorithm: algo, calculated, expected } }; | ||
| } | ||
|
|
||
| // AWS checks x-amz-checksum- first and then x-amz-sdk-checksum-algorithm | ||
| if ('x-amz-sdk-checksum-algorithm' in headers) { | ||
| const sdkAlgo = headers['x-amz-sdk-checksum-algorithm']; | ||
| if (typeof sdkAlgo !== 'string') { | ||
| return { error: ChecksumError.AlgoNotSupportedSDK, details: { algorithm: sdkAlgo } }; | ||
| } | ||
|
|
||
| const sdkLowerAlgo = sdkAlgo.toLowerCase(); | ||
| if (!(sdkLowerAlgo in algorithms)) { | ||
| return { error: ChecksumError.AlgoNotSupportedSDK, details: { algorithm: sdkAlgo } }; | ||
| } | ||
|
|
||
| // If AWS there is a mismatch, AWS returns the same error as if the algo was invalid. | ||
| if (sdkLowerAlgo !== algo) { | ||
| return { error: ChecksumError.AlgoNotSupportedSDK, details: { algorithm: sdkAlgo } }; | ||
| } | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| /** | ||
| * 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) { | ||
| async function validateChecksumsNoChunking(headers, body) { | ||
| if (!headers) { | ||
| return { error: ChecksumError.MissingChecksum, details: null }; | ||
| } | ||
|
|
||
| let md5Present = false; | ||
| if ('content-md5' in headers) { | ||
| if (typeof headers['content-md5'] !== 'string') { | ||
| return { error: ChecksumError.MD5Invalid, details: { expected: headers['content-md5'] } }; | ||
| } | ||
|
|
||
| if (headers['content-md5'].length !== 24) { | ||
| return { error: ChecksumError.MD5Invalid, details: { expected: headers['content-md5'] } }; | ||
| } | ||
|
|
||
| if (!base64Regex.test(headers['content-md5'])) { | ||
| return { error: ChecksumError.MD5Invalid, details: { expected: headers['content-md5'] } }; | ||
| } | ||
|
|
||
| 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'] } }; | ||
| } | ||
|
|
||
| md5Present = true; | ||
| } | ||
|
|
||
| const err = await validateXAmzChecksums(headers, body); | ||
| if (err && err.error === ChecksumError.MissingChecksum && md5Present) { | ||
| // Don't return MissingChecksum if MD5 is present. | ||
| return null; | ||
| } | ||
|
|
||
| return { error: ChecksumError.MissingChecksum, details: null }; | ||
| return err; | ||
| } | ||
|
|
||
| function defaultValidationFunc(request, body, log) { | ||
| const err = validateChecksumsNoChunking(request.headers, body); | ||
| if (err && err.error !== ChecksumError.MissingChecksum) { | ||
| async function defaultValidationFunc(request, body, log) { | ||
| const err = await validateChecksumsNoChunking(request.headers, body); | ||
| if (!err) { | ||
| return null; | ||
| } | ||
|
|
||
| if (err.error !== ChecksumError.MissingChecksum) { | ||
| log.debug('failed checksum validation', { method: request.apiMethod }, err); | ||
| return ArsenalErrors.BadDigest; | ||
| } | ||
|
|
||
| return null; | ||
| switch (err.error) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my suggestion about returning directly errors to avoid having to do this conversion.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The internal errors are have more details than the AWS errors(used for logging), they are also easier to test. |
||
| case ChecksumError.MissingChecksum: | ||
| return null; | ||
| case ChecksumError.XAmzMismatch: { | ||
| const algoUpper = err.details.algorithm.toUpperCase(); | ||
| return ArsenalErrors.BadDigest.customizeDescription( | ||
| `The ${algoUpper} you specified did not match the calculated checksum.` | ||
| ); | ||
| } | ||
| case ChecksumError.AlgoNotSupported: | ||
| return ArsenalErrors.InvalidRequest.customizeDescription( | ||
| 'The algorithm type you specified in x-amz-checksum- header is invalid.' | ||
| ); | ||
| case ChecksumError.AlgoNotSupportedSDK: | ||
| return ArsenalErrors.InvalidRequest.customizeDescription( | ||
| 'Value for x-amz-sdk-checksum-algorithm header is invalid.' | ||
| ); | ||
| case ChecksumError.MissingCorresponding: | ||
| return ArsenalErrors.InvalidRequest.customizeDescription( | ||
| 'x-amz-sdk-checksum-algorithm specified, but no corresponding x-amz-checksum-* ' + | ||
| 'or x-amz-trailer headers were found.' | ||
| ); | ||
| case ChecksumError.MultipleChecksumTypes: | ||
| return ArsenalErrors.InvalidRequest.customizeDescription( | ||
| 'Expecting a single x-amz-checksum- header. Multiple checksum Types are not allowed.' | ||
| ); | ||
| case ChecksumError.MalformedChecksum: | ||
| return ArsenalErrors.InvalidRequest.customizeDescription( | ||
| `Value for x-amz-checksum-${err.details.algorithm} header is invalid.` | ||
| ); | ||
| case ChecksumError.MD5Invalid: | ||
| return ArsenalErrors.InvalidDigest; | ||
| default: | ||
| return ArsenalErrors.BadDigest; | ||
| } | ||
| } | ||
|
|
||
| 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; | ||
| } | ||
| async function validateMethodChecksumNoChunking(request, body, log) { | ||
| if (config.integrityChecks[request.apiMethod] === false) { | ||
| return null; | ||
| } | ||
|
|
||
| return validationFunc(request, body, log); | ||
| if (request.apiMethod in checksumedMethods) { | ||
| return await defaultValidationFunc(request, body, log); | ||
| } | ||
|
|
||
| return null; | ||
|
|
@@ -79,4 +253,5 @@ module.exports = { | |
| ChecksumError, | ||
| validateChecksumsNoChunking, | ||
| validateMethodChecksumNoChunking, | ||
| checksumedMethods, | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of creating internal error types that are not used for any logic nor returned to the client, we could instead directly construct the error when we see it and return it as is with an appropriate descriptive error message (i.e. construct directly an error with
InvalidRequest.customizeDescription(...)). We can keep the special errors that we need special handling for (MissingChecksumetc.)In the unit tests, you can simply check that the error is
InvalidRequest, and if you want to be fancy, also check a part of the error message.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The internal errors are have more details than the AWS errors(used for logging), they are also easier to test.
I prefer having the building of the AMZ error outside the validation function.