Skip to content

Commit ef84fba

Browse files
committed
Merge remote-tracking branch 'origin/bugfix/CLDSRV-620/strip-trailing-checksums' into w/7.70/bugfix/CLDSRV-620/strip-trailing-checksums
2 parents 20060f1 + 7ba8551 commit ef84fba

File tree

9 files changed

+484
-8
lines changed

9 files changed

+484
-8
lines changed

constants.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,13 +200,13 @@ const constants = {
200200
'isDeleteMarker',
201201
],
202202
unsupportedSignatureChecksums: new Set([
203-
'STREAMING-UNSIGNED-PAYLOAD-TRAILER',
204203
'STREAMING-AWS4-HMAC-SHA256-PAYLOAD-TRAILER',
205204
'STREAMING-AWS4-ECDSA-P256-SHA256-PAYLOAD',
206205
'STREAMING-AWS4-ECDSA-P256-SHA256-PAYLOAD-TRAILER',
207206
]),
208207
supportedSignatureChecksums: new Set([
209208
'UNSIGNED-PAYLOAD',
209+
'STREAMING-UNSIGNED-PAYLOAD-TRAILER',
210210
'STREAMING-AWS4-HMAC-SHA256-PAYLOAD',
211211
]),
212212
ipv4Regex: /^(\d{1,3}\.){3}\d{1,3}(\/(3[0-2]|[12]?\d))?$/,

lib/api/apiUtils/object/prepareStream.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
const V4Transform = require('../../../auth/streamingV4/V4Transform');
2+
const TrailingChecksumTransform = require('../../../auth/streamingV4/trailingChecksumTransform');
23

34
/**
45
* Prepares the stream if the chunks are sent in a v4 Auth request
@@ -24,11 +25,25 @@ function prepareStream(stream, streamingV4Params, log, errCb) {
2425
}
2526
const v4Transform = new V4Transform(streamingV4Params, log, errCb);
2627
stream.pipe(v4Transform);
28+
v4Transform.headers = stream.headers;
2729
return v4Transform;
2830
}
2931
return stream;
3032
}
3133

34+
function stripTrailingChecksumStream(stream, log) {
35+
// don't do anything if we are not in the correct integrity check mode
36+
if (stream.headers['x-amz-content-sha256'] !== 'STREAMING-UNSIGNED-PAYLOAD-TRAILER') {
37+
return stream;
38+
}
39+
40+
const trailingChecksumTransform = new TrailingChecksumTransform(log);
41+
stream.pipe(trailingChecksumTransform);
42+
trailingChecksumTransform.headers = stream.headers;
43+
return trailingChecksumTransform;
44+
}
45+
3246
module.exports = {
3347
prepareStream,
48+
stripTrailingChecksumStream,
3449
};

lib/api/apiUtils/object/storeObject.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
const { errors, jsutil } = require('arsenal');
22

33
const { data } = require('../../../data/wrapper');
4-
const { prepareStream } = require('./prepareStream');
4+
const { prepareStream, stripTrailingChecksumStream } = require('./prepareStream');
55

66
/**
77
* Check that `hashedStream.completedHash` matches header `stream.contentMD5`
@@ -58,10 +58,11 @@ function checkHashMatchMD5(stream, hashedStream, dataRetrievalInfo, log, cb) {
5858
function dataStore(objectContext, cipherBundle, stream, size,
5959
streamingV4Params, backendInfo, log, cb) {
6060
const cbOnce = jsutil.once(cb);
61-
const dataStream = prepareStream(stream, streamingV4Params, log, cbOnce);
62-
if (!dataStream) {
61+
const dataStreamTmp = prepareStream(stream, streamingV4Params, log, cbOnce);
62+
if (!dataStreamTmp) {
6363
return process.nextTick(() => cb(errors.InvalidArgument));
6464
}
65+
const dataStream = stripTrailingChecksumStream(dataStreamTmp, log, cbOnce);
6566
return data.put(
6667
cipherBundle, dataStream, size, objectContext, backendInfo, log,
6768
(err, dataRetrievalInfo, hashedStream) => {

lib/api/apiUtils/object/validateChecksumHeaders.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@ const { unsupportedSignatureChecksums, supportedSignatureChecksums } = require('
55
function validateChecksumHeaders(headers) {
66
// If the x-amz-trailer header is present the request is using one of the
77
// trailing checksum algorithms, which are not supported.
8-
if (headers['x-amz-trailer'] !== undefined) {
9-
return errors.BadRequest.customizeDescription('trailing checksum is not supported');
8+
9+
if (headers['x-amz-trailer'] !== undefined &&
10+
headers['x-amz-content-sha256'] !== 'STREAMING-UNSIGNED-PAYLOAD-TRAILER') {
11+
return errors.BadRequest.customizeDescription('signed trailing checksum is not supported');
1012
}
1113

1214
const signatureChecksum = headers['x-amz-content-sha256'];

lib/auth/streamingV4/V4Transform.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ class V4Transform extends Transform {
184184
* @param {Buffer} chunk - chunk from request body
185185
* @param {string} encoding - Data encoding
186186
* @param {function} callback - Callback(err, justDataChunk, encoding)
187-
* @return {function }executes callback with err if applicable
187+
* @return {function} executes callback with err if applicable
188188
*/
189189
_transform(chunk, encoding, callback) {
190190
// 'chunk' here is the node streaming chunk
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
const { Transform } = require('stream');
2+
const { errors } = require('arsenal');
3+
const { maximumAllowedPartSize } = require('../../../constants');
4+
5+
/**
6+
* This class is designed to handle the chunks sent in a streaming
7+
* unsigned playload trailer request. In this iteration, we are not checking
8+
* the checksums, but we are removing them from the stream.
9+
* S3C-9732 will deal with checksum verification.
10+
*/
11+
class TrailingChecksumTransform extends Transform {
12+
/**
13+
* @constructor
14+
* @param {object} log - logger object
15+
*/
16+
constructor(log) {
17+
super({});
18+
this.log = log;
19+
this.chunkSizeBuffer = Buffer.alloc(0);
20+
this.bytesToDiscard = 0; // when trailing \r\n are present, we discard them but they can be in different chunks
21+
this.bytesToRead = 0; // when a chunk is advertised, the size is put here and we forward all bytes
22+
this.streamClosed = false;
23+
}
24+
25+
/**
26+
* This function is executed when there is no more data to be read but before the stream is closed
27+
* We will verify that the trailing checksum structure was upheld
28+
*
29+
* @param {function} callback - Callback(err, data)
30+
* @return {function} executes callback with err if applicable
31+
*/
32+
_flush(callback) {
33+
if (!this.streamClosed) {
34+
this.log.error('stream ended without closing chunked encoding');
35+
return callback(errors.InvalidArgument);
36+
}
37+
return callback();
38+
}
39+
40+
/**
41+
* This function will remove the trailing checksum from the stream
42+
*
43+
* @param {Buffer} chunkInput - chunk from request body
44+
* @param {string} encoding - Data encoding
45+
* @param {function} callback - Callback(err, justDataChunk, encoding)
46+
* @return {function} executes callback with err if applicable
47+
*/
48+
_transform(chunkInput, encoding, callback) {
49+
let chunk = chunkInput;
50+
while (chunk.byteLength > 0 && !this.streamClosed) {
51+
if (this.bytesToDiscard > 0) {
52+
const toDiscard = Math.min(this.bytesToDiscard, chunk.byteLength);
53+
chunk = chunk.subarray(toDiscard);
54+
this.bytesToDiscard -= toDiscard;
55+
continue;
56+
}
57+
// forward up to bytesToRead bytes from the chunk, restart processing on leftover
58+
if (this.bytesToRead > 0) {
59+
const toRead = Math.min(this.bytesToRead, chunk.byteLength);
60+
this.push(chunk.subarray(0, toRead));
61+
chunk = chunk.subarray(toRead);
62+
this.bytesToRead -= toRead;
63+
if (this.bytesToRead === 0) {
64+
this.bytesToDiscard = 2;
65+
}
66+
continue;
67+
}
68+
69+
// we are now looking for the chunk size field
70+
// no need to look further than 10 bytes since the field cannot be bigger: the max
71+
// chunk size is 5GB (see constants.maximumAllowedPartSize)
72+
const lineBreakIndex = chunk.subarray(0, 10).indexOf('\r');
73+
const bytesToKeep = lineBreakIndex === -1 ? chunk.byteLength : lineBreakIndex;
74+
if (this.chunkSizeBuffer.byteLength + bytesToKeep > 10) {
75+
this.log.error('chunk size field too big', {
76+
chunkSizeBuffer: this.chunkSizeBuffer.subarray(0, 11).toString('hex'),
77+
chunkSizeBufferLength: this.chunkSizeBuffer.length,
78+
truncatedChunk: chunk.subarray(0, 10).toString('hex'),
79+
});
80+
// if bigger, the chunk would be over 5 GB
81+
// returning early to avoid a DoS by memory exhaustion
82+
return callback(errors.InvalidArgument);
83+
}
84+
if (lineBreakIndex === -1) {
85+
// no delimiter, we'll keep the chunk for later
86+
this.chunkSizeBuffer = Buffer.concat([this.chunkSizeBuffer, chunk]);
87+
return callback();
88+
}
89+
90+
this.chunkSizeBuffer = Buffer.concat([this.chunkSizeBuffer, chunk.subarray(0, lineBreakIndex)]);
91+
chunk = chunk.subarray(lineBreakIndex);
92+
93+
// chunk-size is sent in hex
94+
const chunkSizeStr = this.chunkSizeBuffer.toString();
95+
const dataSize = parseInt(chunkSizeStr, 16);
96+
// we check that the parsing is correct (parseInt returns a partial parse when it fails)
97+
if (isNaN(dataSize) || dataSize.toString(16) !== chunkSizeStr.toLowerCase()) {
98+
this.log.error('invalid chunk size', { chunkSizeBuffer: chunkSizeStr });
99+
return callback(errors.InvalidArgument);
100+
}
101+
this.chunkSizeBuffer = Buffer.alloc(0);
102+
if (dataSize === 0) {
103+
// TODO: check if the checksum is correct (S3C-9732)
104+
// last chunk, no more data to read, the stream is closed
105+
this.streamClosed = true;
106+
}
107+
if (dataSize > maximumAllowedPartSize) {
108+
this.log.error('chunk size too big', { dataSize });
109+
return callback(errors.EntityTooLarge);
110+
}
111+
this.bytesToRead = dataSize;
112+
this.bytesToDiscard = 2;
113+
}
114+
115+
return callback();
116+
}
117+
}
118+
119+
module.exports = TrailingChecksumTransform;

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "s3",
3-
"version": "7.70.62",
3+
"version": "7.70.63",
44
"description": "S3 connector",
55
"main": "index.js",
66
"engines": {
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
const assert = require('assert');
2+
const async = require('async');
3+
const { makeS3Request } = require('../utils/makeRequest');
4+
const HttpRequestAuthV4 = require('../utils/HttpRequestAuthV4');
5+
6+
const bucket = 'testunsupportedchecksumsbucket';
7+
const objectKey = 'key';
8+
const objData = Buffer.alloc(1024, 'a');
9+
// note this is not the correct checksum in objDataWithTrailingChecksum
10+
const objDataWithTrailingChecksum = '10\r\n0123456789abcdef\r\n' +
11+
'10\r\n0123456789abcdef\r\n' +
12+
'0\r\nx-amz-checksum-crc64nvme:YeIDuLa7tU0=\r\n';
13+
const objDataWithoutTrailingChecksum = '0123456789abcdef0123456789abcdef';
14+
15+
const config = require('../../config.json');
16+
const authCredentials = {
17+
accessKey: config.accessKey,
18+
secretKey: config.secretKey,
19+
};
20+
21+
const itSkipIfAWS = process.env.AWS_ON_AIR ? it.skip : it;
22+
23+
describe('trailing checksum requests:', () => {
24+
before(done => {
25+
makeS3Request({
26+
method: 'PUT',
27+
authCredentials,
28+
bucket,
29+
}, err => {
30+
assert.ifError(err);
31+
done();
32+
});
33+
});
34+
35+
after(done => {
36+
async.series([
37+
next => makeS3Request({
38+
method: 'DELETE',
39+
authCredentials,
40+
bucket,
41+
objectKey,
42+
}, next),
43+
next => makeS3Request({
44+
method: 'DELETE',
45+
authCredentials,
46+
bucket,
47+
}, next),
48+
], err => {
49+
assert.ifError(err);
50+
done();
51+
});
52+
});
53+
54+
it('should accept unsigned trailing checksum', done => {
55+
const req = new HttpRequestAuthV4(
56+
`http://localhost:8000/${bucket}/${objectKey}`,
57+
Object.assign(
58+
{
59+
method: 'PUT',
60+
headers: {
61+
'content-length': objDataWithTrailingChecksum.length,
62+
'x-amz-decoded-content-length': objDataWithoutTrailingChecksum.length,
63+
'x-amz-content-sha256': 'STREAMING-UNSIGNED-PAYLOAD-TRAILER',
64+
'x-amz-trailer': 'x-amz-checksum-crc64nvme',
65+
},
66+
},
67+
authCredentials
68+
),
69+
res => {
70+
assert.strictEqual(res.statusCode, 200);
71+
res.on('data', () => {});
72+
res.on('end', done);
73+
}
74+
);
75+
76+
req.on('error', err => {
77+
assert.ifError(err);
78+
});
79+
80+
req.write(objDataWithTrailingChecksum);
81+
82+
req.once('drain', () => {
83+
req.end();
84+
});
85+
});
86+
87+
it('should have correct object content for unsigned trailing checksum', done => {
88+
makeS3Request({
89+
method: 'GET',
90+
authCredentials,
91+
bucket,
92+
objectKey,
93+
}, (err, res) => {
94+
assert.ifError(err);
95+
assert.strictEqual(res.statusCode, 200);
96+
// check that the object data is the input stripped of the trailing checksum
97+
assert.strictEqual(res.body, objDataWithoutTrailingChecksum);
98+
return done();
99+
});
100+
});
101+
102+
itSkipIfAWS('should respond with BadRequest for signed trailing checksum', done => {
103+
const req = new HttpRequestAuthV4(
104+
`http://localhost:8000/${bucket}/${objectKey}`,
105+
Object.assign(
106+
{
107+
method: 'PUT',
108+
headers: {
109+
'content-length': objData.length,
110+
'x-amz-content-sha256': 'STREAMING-AWS4-HMAC-SHA256-PAYLOAD-TRAILER',
111+
'x-amz-trailer': 'x-amz-checksum-sha256',
112+
},
113+
},
114+
authCredentials
115+
),
116+
res => {
117+
assert.strictEqual(res.statusCode, 400);
118+
res.on('data', () => {});
119+
res.on('end', done);
120+
}
121+
);
122+
123+
req.on('error', err => {
124+
assert.ifError(err);
125+
});
126+
127+
req.write(objData);
128+
129+
req.once('drain', () => {
130+
req.end();
131+
});
132+
});
133+
});

0 commit comments

Comments
 (0)