CLDSRV-620: Ignore trailing checksums in upload requests

fredmnl · fredmnl · commit a9afd5fa7483 · 2025-03-07T15:29:30.000+01:00
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/tests/unit/auth/TrailingChecksumTransform.js b/tests/unit/auth/TrailingChecksumTransform.js
