diff --git a/constants.js b/constants.js index 09cbe799c0..c547c44dd6 100644 --- a/constants.js +++ b/constants.js @@ -187,13 +187,13 @@ const constants = { // Session name of the backbeat lifecycle assumed role session. backbeatLifecycleSessionName: 'backbeat-lifecycle', unsupportedSignatureChecksums: new Set([ - 'STREAMING-UNSIGNED-PAYLOAD-TRAILER', 'STREAMING-AWS4-HMAC-SHA256-PAYLOAD-TRAILER', 'STREAMING-AWS4-ECDSA-P256-SHA256-PAYLOAD', 'STREAMING-AWS4-ECDSA-P256-SHA256-PAYLOAD-TRAILER', ]), supportedSignatureChecksums: new Set([ 'UNSIGNED-PAYLOAD', + 'STREAMING-UNSIGNED-PAYLOAD-TRAILER', 'STREAMING-AWS4-HMAC-SHA256-PAYLOAD', ]), ipv4Regex: /^(\d{1,3}\.){3}\d{1,3}(\/(3[0-2]|[12]?\d))?$/, diff --git a/lib/api/apiUtils/object/prepareStream.js b/lib/api/apiUtils/object/prepareStream.js index 7d436dd96b..08a1947695 100644 --- a/lib/api/apiUtils/object/prepareStream.js +++ b/lib/api/apiUtils/object/prepareStream.js @@ -1,4 +1,5 @@ const V4Transform = require('../../../auth/streamingV4/V4Transform'); +const TrailingChecksumTransform = require('../../../auth/streamingV4/trailingChecksumTransform'); /** * Prepares the stream if the chunks are sent in a v4 Auth request @@ -24,11 +25,26 @@ function prepareStream(stream, streamingV4Params, log, errCb) { } const v4Transform = new V4Transform(streamingV4Params, log, errCb); stream.pipe(v4Transform); + v4Transform.headers = stream.headers; return v4Transform; } return stream; } +function stripTrailingChecksumStream(stream, log, errCb) { + // don't do anything if x-amz-trailer has not been set + if (stream.headers['x-amz-trailer'] === undefined || + stream.headers['x-amz-trailer'] === '') { + return stream; + } + + const trailingChecksumTransform = new TrailingChecksumTransform(log, errCb); + stream.pipe(trailingChecksumTransform); + trailingChecksumTransform.headers = stream.headers; + return trailingChecksumTransform; +} + module.exports = { prepareStream, + stripTrailingChecksumStream, }; diff --git a/lib/api/apiUtils/object/storeObject.js b/lib/api/apiUtils/object/storeObject.js index 346683e66c..8beea03ecb 100644 --- a/lib/api/apiUtils/object/storeObject.js +++ b/lib/api/apiUtils/object/storeObject.js @@ -1,7 +1,7 @@ const { errors, jsutil } = require('arsenal'); const { data } = require('../../../data/wrapper'); -const { prepareStream } = require('./prepareStream'); +const { prepareStream, stripTrailingChecksumStream } = require('./prepareStream'); /** * Check that `hashedStream.completedHash` matches header `stream.contentMD5` @@ -58,10 +58,11 @@ function checkHashMatchMD5(stream, hashedStream, dataRetrievalInfo, log, cb) { function dataStore(objectContext, cipherBundle, stream, size, streamingV4Params, backendInfo, log, cb) { const cbOnce = jsutil.once(cb); - const dataStream = prepareStream(stream, streamingV4Params, log, cbOnce); - if (!dataStream) { + const dataStreamTmp = prepareStream(stream, streamingV4Params, log, cbOnce); + if (!dataStreamTmp) { return process.nextTick(() => cb(errors.InvalidArgument)); } + const dataStream = stripTrailingChecksumStream(dataStreamTmp, log, cbOnce); return data.put( cipherBundle, dataStream, size, objectContext, backendInfo, log, (err, dataRetrievalInfo, hashedStream) => { diff --git a/lib/api/apiUtils/object/validateChecksumHeaders.js b/lib/api/apiUtils/object/validateChecksumHeaders.js index 8162603695..90c5041516 100644 --- a/lib/api/apiUtils/object/validateChecksumHeaders.js +++ b/lib/api/apiUtils/object/validateChecksumHeaders.js @@ -5,8 +5,10 @@ const { unsupportedSignatureChecksums, supportedSignatureChecksums } = require(' function validateChecksumHeaders(headers) { // If the x-amz-trailer header is present the request is using one of the // trailing checksum algorithms, which are not supported. - if (headers['x-amz-trailer'] !== undefined) { - return errors.BadRequest.customizeDescription('trailing checksum is not supported'); + + if (headers['x-amz-trailer'] !== undefined && + headers['x-amz-content-sha256'] !== 'STREAMING-UNSIGNED-PAYLOAD-TRAILER') { + return errors.BadRequest.customizeDescription('signed trailing checksum is not supported'); } const signatureChecksum = headers['x-amz-content-sha256']; diff --git a/lib/auth/streamingV4/V4Transform.js b/lib/auth/streamingV4/V4Transform.js index 0533d7c7a0..e28d6efcfb 100644 --- a/lib/auth/streamingV4/V4Transform.js +++ b/lib/auth/streamingV4/V4Transform.js @@ -184,7 +184,7 @@ class V4Transform extends Transform { * @param {Buffer} chunk - chunk from request body * @param {string} encoding - Data encoding * @param {function} callback - Callback(err, justDataChunk, encoding) - * @return {function }executes callback with err if applicable + * @return {function} executes callback with err if applicable */ _transform(chunk, encoding, callback) { // 'chunk' here is the node streaming chunk diff --git a/lib/auth/streamingV4/trailingChecksumTransform.js b/lib/auth/streamingV4/trailingChecksumTransform.js new file mode 100644 index 0000000000..b2df4527b1 --- /dev/null +++ b/lib/auth/streamingV4/trailingChecksumTransform.js @@ -0,0 +1,100 @@ +const { Transform } = require('stream'); +const { errors } = require('arsenal'); + +/** + * This class is designed to handle the chunks sent in a streaming + * v4 Auth request + */ +class TrailingChecksumTransform extends Transform { + /** + * @constructor + * @param {object} log - logger object + * @param {function} errCb - callback called if an error occurs + */ + constructor(log, errCb) { + super({}); + this.log = log; + this.errCb = errCb; + this.chunkSizeBuffer = Buffer.alloc(0); + this.bytesToDiscard = 0; // when trailing \r\n are present, we discard them but they can be in different chunks + this.bytesToRead = 0; // when a chunk is advertised, the size is put here and we forward all bytes + this.streamClosed = false; + } + + /** + * This function will remove the trailing checksum from the stream + * + * @param {Buffer} chunkInput - chunk from request body + * @param {string} encoding - Data encoding + * @param {function} callback - Callback(err, justDataChunk, encoding) + * @return {function} executes callback with err if applicable + */ + _transform(chunkInput, encoding, callback) { + let chunk = chunkInput; + while (chunk.byteLength > 0 && !this.streamClosed) { + if (this.bytesToDiscard > 0) { + const toDiscard = Math.min(this.bytesToDiscard, chunk.byteLength); + chunk = chunk.subarray(toDiscard); + this.bytesToDiscard -= toDiscard; + continue; + } + // forward up to bytesToRead bytes from the chunk, restart processing on leftover + if (this.bytesToRead > 0) { + const toRead = Math.min(this.bytesToRead, chunk.byteLength); + this.push(chunk.subarray(0, toRead)); + chunk = chunk.subarray(toRead); + this.bytesToRead -= toRead; + if (this.bytesToRead === 0) { + this.bytesToDiscard = 2; + } + continue; + } + + const lineBreakIndex = chunk.indexOf('\r'); + if (lineBreakIndex === -1) { + if (this.chunkSizeBuffer.byteLength + chunk.byteLength > 10) { + this.log.info('chunk size field too big', { + chunkSizeBuffer: this.chunkSizeBuffer.toString(), + truncatedChunk: chunk.subarray(0, 16).toString(), + }); + // if bigger, the chunk would be over 5 GB + // returning early to avoid a DoS by memory exhaustion + return callback(errors.InvalidArgument); + } + // no delimiter, we'll keep the chunk for later + this.chunkSizeBuffer = Buffer.concat([this.chunkSizeBuffer, chunk]); + return callback(); + } + + this.chunkSizeBuffer = Buffer.concat([this.chunkSizeBuffer, chunk.subarray(0, lineBreakIndex)]); + chunk = chunk.subarray(lineBreakIndex); + + // chunk-size is sent in hex + if (!/^[0-9a-fA-F]+$/.test(this.chunkSizeBuffer.toString())) { + this.log.info('chunk size is not a valid hex number', { + chunkSizeBuffer: this.chunkSizeBuffer.toString(), + }); + return callback(errors.InvalidArgument); + } + const dataSize = Number.parseInt(this.chunkSizeBuffer.toString(), 16); + if (Number.isNaN(dataSize)) { + this.log.error('unable to parse chunk size', { + chunkSizeBuffer: this.chunkSizeBuffer.toString(), + }); + return callback(errors.InvalidArgument); + } + this.chunkSizeBuffer = Buffer.alloc(0); + if (dataSize === 0) { + // TODO: check if the checksum is correct + // last chunk, no more data to read, the stream is closed + this.streamClosed = true; + } + this.bytesToRead = dataSize; + this.bytesToDiscard = 2; + } + + return callback(); + } +} + +module.exports = TrailingChecksumTransform; diff --git a/tests/functional/raw-node/test/trailingChecksums.js b/tests/functional/raw-node/test/trailingChecksums.js new file mode 100644 index 0000000000..77131270aa --- /dev/null +++ b/tests/functional/raw-node/test/trailingChecksums.js @@ -0,0 +1,132 @@ +const assert = require('assert'); +const async = require('async'); +const { makeS3Request } = require('../utils/makeRequest'); +const HttpRequestAuthV4 = require('../utils/HttpRequestAuthV4'); + +const bucket = 'testunsupportedchecksumsbucket'; +const objectKey = 'key'; +const objData = Buffer.alloc(1024, 'a'); +// note this is not the correct checksum in objDataWithTrailingChecksum +const objDataWithTrailingChecksum = '10\r\n0123456789abcdef\r\n' + + '10\r\n0123456789abcdef\r\n' + + '0\r\nx-amz-checksum-crc64nvme:YeIDuLa7tU0=\r\n'; +const objDataWithoutTrailingChecksum = '0123456789abcdef0123456789abcdef'; + +const authCredentials = { + accessKey: 'accessKey1', + secretKey: 'verySecretKey1', +}; + +const itSkipIfAWS = process.env.AWS_ON_AIR ? it.skip : it; + +describe('trailing checksum requests:', () => { + before(done => { + makeS3Request({ + method: 'PUT', + authCredentials, + bucket, + }, err => { + assert.ifError(err); + done(); + }); + }); + + after(done => { + async.series([ + next => makeS3Request({ + method: 'DELETE', + authCredentials, + bucket, + objectKey, + }, next), + next => makeS3Request({ + method: 'DELETE', + authCredentials, + bucket, + }, next), + ], err => { + assert.ifError(err); + done(); + }); + }); + + it('should accept unsigned trailing checksum', done => { + const req = new HttpRequestAuthV4( + `http://localhost:8000/${bucket}/${objectKey}`, + Object.assign( + { + method: 'PUT', + headers: { + 'content-length': objDataWithTrailingChecksum.length, + 'x-amz-decoded-content-length': objDataWithoutTrailingChecksum.length, + 'x-amz-content-sha256': 'STREAMING-UNSIGNED-PAYLOAD-TRAILER', + 'x-amz-trailer': 'x-amz-checksum-crc64nvme', + }, + }, + authCredentials + ), + res => { + assert.strictEqual(res.statusCode, 200); + res.on('data', () => {}); + res.on('end', done); + } + ); + + req.on('error', err => { + assert.ifError(err); + }); + + req.write(objDataWithTrailingChecksum); + + req.once('drain', () => { + req.end(); + }); + }); + + it('should have correct object content for unsigned trailing checksum', done => { + makeS3Request({ + method: 'GET', + authCredentials, + bucket, + objectKey, + }, (err, res) => { + assert.ifError(err); + assert.strictEqual(res.statusCode, 200); + // check that the object data is the input stripped of the trailing checksum + assert.strictEqual(res.body, objDataWithoutTrailingChecksum); + return done(); + }); + }); + + itSkipIfAWS('should respond with BadRequest for signed trailing checksum', done => { + const req = new HttpRequestAuthV4( + `http://localhost:8000/${bucket}/${objectKey}`, + Object.assign( + { + method: 'PUT', + headers: { + 'content-length': objData.length, + 'x-amz-content-sha256': 'STREAMING-AWS4-HMAC-SHA256-PAYLOAD-TRAILER', + 'x-amz-trailer': 'x-amz-checksum-sha256', + }, + }, + authCredentials + ), + res => { + assert.strictEqual(res.statusCode, 400); + res.on('data', () => {}); + res.on('end', done); + } + ); + + req.on('error', err => { + assert.ifError(err); + }); + + req.write(objData); + + req.once('drain', () => { + req.end(); + }); + }); +}); diff --git a/tests/functional/raw-node/test/unsupportedChecksums.js b/tests/functional/raw-node/test/unsupportedChecksums.js deleted file mode 100644 index dd04be0f5f..0000000000 --- a/tests/functional/raw-node/test/unsupportedChecksums.js +++ /dev/null @@ -1,70 +0,0 @@ -const assert = require('assert'); -const { makeS3Request } = require('../utils/makeRequest'); -const HttpRequestAuthV4 = require('../utils/HttpRequestAuthV4'); - -const bucket = 'testunsupportedchecksumsbucket'; -const objectKey = 'key'; -const objData = Buffer.alloc(1024, 'a'); - -const authCredentials = { - accessKey: 'accessKey1', - secretKey: 'verySecretKey1', -}; - -const itSkipIfAWS = process.env.AWS_ON_AIR ? it.skip : it; - -describe('unsupported checksum requests:', () => { - before(done => { - makeS3Request({ - method: 'PUT', - authCredentials, - bucket, - }, err => { - assert.ifError(err); - done(); - }); - }); - - after(done => { - makeS3Request({ - method: 'DELETE', - authCredentials, - bucket, - }, err => { - assert.ifError(err); - done(); - }); - }); - - itSkipIfAWS('should respond with BadRequest for trailing checksum', done => { - const req = new HttpRequestAuthV4( - `http://localhost:8000/${bucket}/${objectKey}`, - Object.assign( - { - method: 'PUT', - headers: { - 'content-length': objData.length, - 'x-amz-content-sha256': 'STREAMING-AWS4-HMAC-SHA256-PAYLOAD-TRAILER', - 'x-amz-trailer': 'x-amz-checksum-sha256', - }, - }, - authCredentials - ), - res => { - assert.strictEqual(res.statusCode, 400); - res.on('data', () => {}); - res.on('end', done); - } - ); - - req.on('error', err => { - assert.ifError(err); - }); - - req.write(objData); - - req.once('drain', () => { - req.end(); - }); - }); -}); diff --git a/tests/unit/auth/TrailingChecksumTransform.js b/tests/unit/auth/TrailingChecksumTransform.js new file mode 100644 index 0000000000..fc011cf2a2 --- /dev/null +++ b/tests/unit/auth/TrailingChecksumTransform.js @@ -0,0 +1,131 @@ +const assert = require('assert'); +const async = require('async'); +const { Readable } = require('stream'); + +const TrailingChecksumTransform = require('../../../lib/auth/streamingV4/trailingChecksumTransform'); +const { DummyRequestLogger } = require('../helpers'); + +const log = new DummyRequestLogger(); + +// note this is not the correct checksum in objDataWithTrailingChecksum +const objDataWithTrailingChecksum = '10\r\n0123456789abcdef\r\n' + + '2\r\n01\r\n' + + '1\r\n2\r\n' + + 'd\r\n3456789abcdef\r\n' + + '0\r\nchecksum:xyz=\r\n'; +const objDataWithoutTrailingChecksum = '0123456789abcdef0123456789abcdef'; + +class ChunkedReader extends Readable { + constructor(chunks) { + super(); + this._parts = chunks; + this._index = 0; + } + + _read() { + if (this._index >= this._parts.length) { + this.push(null); + return; + } + this.push(this._parts[this._index]); + this._index++; + } +} + +describe('TrailingChecksumTransform class', () => { + it('should correctly remove checksums', done => { + const trailingChecksumTransform = new TrailingChecksumTransform(log, err => { + assert.strictEqual(err, null); + }); + const chunks = [ + Buffer.from(objDataWithTrailingChecksum), + ]; + const chunkedReader = new ChunkedReader(chunks); + chunkedReader.pipe(trailingChecksumTransform); + const outputChunks = []; + trailingChecksumTransform.on('data', (chunk) => outputChunks.push(Buffer.from(chunk))); + trailingChecksumTransform.on('finish', () => { + const data = Buffer.concat(outputChunks).toString(); + assert.strictEqual(data, objDataWithoutTrailingChecksum); + done(); + }); + trailingChecksumTransform.on('error', err => { + assert.ifError(err); + }); + }); + + // test all bisection of the input string + async.forEach([...Array(objDataWithTrailingChecksum.length).keys()], (i) => { + it(`should correctly remove checksums, cut at ${i}`, done => { + const trailingChecksumTransform = new TrailingChecksumTransform(log, err => { + assert.strictEqual(err, null); + }); + const chunks = [ + Buffer.from(objDataWithTrailingChecksum.substring(0, i)), + Buffer.from(objDataWithTrailingChecksum.substring(i)), + ]; + const chunkedReader = new ChunkedReader(chunks); + chunkedReader.pipe(trailingChecksumTransform); + const outputChunks = []; + trailingChecksumTransform.on('data', (chunk) => outputChunks.push(Buffer.from(chunk))); + trailingChecksumTransform.on('finish', () => { + const data = Buffer.concat(outputChunks).toString(); + assert.strictEqual(data, objDataWithoutTrailingChecksum); + done(); + }); + trailingChecksumTransform.on('error', err => { + assert.ifError(err); + }); + }); + }); + + // test all trisection of the input string + async.forEach([...Array(objDataWithTrailingChecksum.length - 2).keys()], (i) => { + async.forEach([...Array(objDataWithTrailingChecksum.length - i - 2).keys()], (j) => { + it(`should correctly remove checksums, cut at ${i} and ${i + j + 1}`, done => { + const trailingChecksumTransform = new TrailingChecksumTransform(log, err => { + assert.strictEqual(err, null); + }); + const chunks = [ + Buffer.from(objDataWithTrailingChecksum.substring(0, i + 1)), + Buffer.from(objDataWithTrailingChecksum.substring(i + 1, i + j + 2)), + Buffer.from(objDataWithTrailingChecksum.substring(i + j + 2)), + ]; + const chunkedReader = new ChunkedReader(chunks); + chunkedReader.pipe(trailingChecksumTransform); + const outputChunks = []; + trailingChecksumTransform.on('data', (chunk) => outputChunks.push(Buffer.from(chunk))); + trailingChecksumTransform.on('finish', () => { + const data = Buffer.concat(outputChunks).toString(); + assert.strictEqual(data, objDataWithoutTrailingChecksum); + done(); + }); + trailingChecksumTransform.on('error', err => { + assert.ifError(err); + }); + }); + }); + }); + + it('should correctly remove checksums, cut at each individual byte', done => { + const trailingChecksumTransform = new TrailingChecksumTransform(log, err => { + assert.strictEqual(err, null); + }); + const chunks = []; + for (let i = 0; i < objDataWithTrailingChecksum.length; i++) { + chunks.push(objDataWithTrailingChecksum.substring(i, i + 1)); + } + const chunkedReader = new ChunkedReader(chunks); + chunkedReader.pipe(trailingChecksumTransform); + const outputChunks = []; + trailingChecksumTransform.on('data', (chunk) => outputChunks.push(Buffer.from(chunk))); + trailingChecksumTransform.on('finish', () => { + const data = Buffer.concat(outputChunks).toString(); + assert.strictEqual(data, objDataWithoutTrailingChecksum); + done(); + }); + trailingChecksumTransform.on('error', err => { + assert.ifError(err); + }); + }); +});