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..493e25f185 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,25 @@ 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) { + // don't do anything if we are not in the correct integrity check mode + if (stream.headers['x-amz-content-sha256'] !== 'STREAMING-UNSIGNED-PAYLOAD-TRAILER') { + return stream; + } + + const trailingChecksumTransform = new TrailingChecksumTransform(log); + 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..48870c80e7 --- /dev/null +++ b/lib/auth/streamingV4/trailingChecksumTransform.js @@ -0,0 +1,119 @@ +const { Transform } = require('stream'); +const { errors } = require('arsenal'); +const { maximumAllowedPartSize } = require('../../../constants'); + +/** + * This class is designed to handle the chunks sent in a streaming + * unsigned playload trailer request. In this iteration, we are not checking + * the checksums, but we are removing them from the stream. + * S3C-9732 will deal with checksum verification. + */ +class TrailingChecksumTransform extends Transform { + /** + * @constructor + * @param {object} log - logger object + */ + constructor(log) { + super({}); + this.log = log; + 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 is executed when there is no more data to be read but before the stream is closed + * We will verify that the trailing checksum structure was upheld + * + * @param {function} callback - Callback(err, data) + * @return {function} executes callback with err if applicable + */ + _flush(callback) { + if (!this.streamClosed) { + this.log.error('stream ended without closing chunked encoding'); + return callback(errors.InvalidArgument); + } + return callback(); + } + + /** + * 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; + } + + // we are now looking for the chunk size field + // no need to look further than 10 bytes since the field cannot be bigger: the max + // chunk size is 5GB (see constants.maximumAllowedPartSize) + const lineBreakIndex = chunk.subarray(0, 10).indexOf('\r'); + const bytesToKeep = lineBreakIndex === -1 ? chunk.byteLength : lineBreakIndex; + if (this.chunkSizeBuffer.byteLength + bytesToKeep > 10) { + this.log.error('chunk size field too big', { + chunkSizeBuffer: this.chunkSizeBuffer.subarray(0, 11).toString('hex'), + chunkSizeBufferLength: this.chunkSizeBuffer.length, + truncatedChunk: chunk.subarray(0, 10).toString('hex'), + }); + // if bigger, the chunk would be over 5 GB + // returning early to avoid a DoS by memory exhaustion + return callback(errors.InvalidArgument); + } + if (lineBreakIndex === -1) { + // 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 + const chunkSizeStr = this.chunkSizeBuffer.toString(); + const dataSize = parseInt(chunkSizeStr, 16); + // we check that the parsing is correct (parseInt returns a partial parse when it fails) + if (isNaN(dataSize) || dataSize.toString(16) !== chunkSizeStr.toLowerCase()) { + this.log.error('invalid chunk size', { chunkSizeBuffer: chunkSizeStr }); + return callback(errors.InvalidArgument); + } + this.chunkSizeBuffer = Buffer.alloc(0); + if (dataSize === 0) { + // TODO: check if the checksum is correct (S3C-9732) + // last chunk, no more data to read, the stream is closed + this.streamClosed = true; + } + if (dataSize > maximumAllowedPartSize) { + this.log.error('chunk size too big', { dataSize }); + return callback(errors.EntityTooLarge); + } + this.bytesToRead = dataSize; + this.bytesToDiscard = 2; + } + + return callback(); + } +} + +module.exports = TrailingChecksumTransform; diff --git a/package.json b/package.json index 4eb2151298..ecefbae9e3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "s3", - "version": "7.10.52", + "version": "7.10.53", "description": "S3 connector", "main": "index.js", "engines": { diff --git a/tests/functional/raw-node/test/trailingChecksums.js b/tests/functional/raw-node/test/trailingChecksums.js new file mode 100644 index 0000000000..bad429c3c8 --- /dev/null +++ b/tests/functional/raw-node/test/trailingChecksums.js @@ -0,0 +1,133 @@ +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 config = require('../../config.json'); +const authCredentials = { + accessKey: config.accessKey, + secretKey: config.secretKey, +}; + +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/unit/auth/TrailingChecksumTransform.js b/tests/unit/auth/TrailingChecksumTransform.js new file mode 100644 index 0000000000..9cae54e659 --- /dev/null +++ b/tests/unit/auth/TrailingChecksumTransform.js @@ -0,0 +1,206 @@ +const { errors } = require('arsenal'); +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\n01234\r6789abcd\r\n\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 = '01234\r6789abcd\r\n0123456789abcdef'; + +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++; + } + + getIndex() { + return this._index; + } +} + +describe('TrailingChecksumTransform class', () => { + it('should correctly remove checksums', done => { + const trailingChecksumTransform = new TrailingChecksumTransform(log); + trailingChecksumTransform.on('error', 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); + trailingChecksumTransform.on('error', 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); + trailingChecksumTransform.on('error', 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); + trailingChecksumTransform.on('error', 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); + }); + }); + + it('should return an error if the format does not follow trailing checksum specification', done => { + const trailingChecksumTransform = new TrailingChecksumTransform(log); + const chunks = [ + Buffer.from('11\r\n'), + Buffer.alloc(1000000), + Buffer.alloc(1000000), + Buffer.alloc(1000000), + Buffer.alloc(1000000), + Buffer.alloc(1000000), + Buffer.alloc(1000000), + Buffer.alloc(1000000), + Buffer.alloc(1000000), + ]; + const chunkedReader = new ChunkedReader(chunks); + trailingChecksumTransform.on('error', err => { + assert.deepStrictEqual(err, errors.InvalidArgument); + trailingChecksumTransform.end(); + }); + let bytesWritten = 0; + trailingChecksumTransform.on('data', chunk => { + bytesWritten += chunk.length; + }); + trailingChecksumTransform.on('close', () => { + assert.equal(bytesWritten, 17); + // 2 is the minimum but it looks like buffering will fetch additional chunks before the error is emitted + // as long as we do not read the full input stream, this is fine + assert.ok(chunkedReader.getIndex() <= 4); + done(); + }); + chunkedReader.pipe(trailingChecksumTransform); + }); + + it('should return early if supplied with an out of specification chunk size', done => { + const trailingChecksumTransform = new TrailingChecksumTransform(log); + const chunks = [ + Buffer.from('500000'), + Buffer.from('000000\r\n'), + Buffer.alloc(1000000), + Buffer.alloc(1000000), + Buffer.alloc(1000000), + Buffer.alloc(1000000), + Buffer.alloc(1000000), + Buffer.alloc(1000000), + Buffer.alloc(1000000), + Buffer.alloc(1000000), + Buffer.alloc(1000000), + ]; + const chunkedReader = new ChunkedReader(chunks); + trailingChecksumTransform.on('error', err => { + assert.deepStrictEqual(err, errors.InvalidArgument); + trailingChecksumTransform.end(); + }); + let bytesWritten = 0; + trailingChecksumTransform.on('data', chunk => { + bytesWritten += chunk.length; + }); + trailingChecksumTransform.on('close', () => { + assert.equal(bytesWritten, 0); + // 2 is the minimum but it looks like buffering will fetch additional chunks before the error is emitted + // as long as we do not read the full input stream, this is fine + assert.ok(chunkedReader.getIndex() <= 4); + done(); + }); + chunkedReader.pipe(trailingChecksumTransform); + }); +});