Skip to content

Commit 7db1258

Browse files
fredmnlfredmnl
committed
S3C-9769: Ignore trailing checksums in upload requests
1 parent a6c4875 commit 7db1258

File tree

8 files changed

+266
-77
lines changed

8 files changed

+266
-77
lines changed

‎constants.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,13 +187,13 @@ const constants = {
187187
// Session name of the backbeat lifecycle assumed role session.
188188
backbeatLifecycleSessionName: 'backbeat-lifecycle',
189189
unsupportedSignatureChecksums: new Set([
190-
'STREAMING-UNSIGNED-PAYLOAD-TRAILER',
191190
'STREAMING-AWS4-HMAC-SHA256-PAYLOAD-TRAILER',
192191
'STREAMING-AWS4-ECDSA-P256-SHA256-PAYLOAD',
193192
'STREAMING-AWS4-ECDSA-P256-SHA256-PAYLOAD-TRAILER',
194193
]),
195194
supportedSignatureChecksums: new Set([
196195
'UNSIGNED-PAYLOAD',
196+
'STREAMING-UNSIGNED-PAYLOAD-TRAILER',
197197
'STREAMING-AWS4-HMAC-SHA256-PAYLOAD',
198198
]),
199199
ipv4Regex: /^(\d{1,3}\.){3}\d{1,3}(\/(3[0-2]|[12]?\d))?$/,

‎lib/api/apiUtils/object/prepareStream.js‎

Lines changed: 16 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,26 @@ 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, errCb) {
35+
// don't do anything if x-amz-trailer has not been set
36+
if (stream.headers['x-amz-trailer'] === undefined ||
37+
stream.headers['x-amz-trailer'] === '') {
38+
return stream;
39+
}
40+
41+
const trailingChecksumTransform = new TrailingChecksumTransform(log, errCb);
42+
stream.pipe(trailingChecksumTransform);
43+
trailingChecksumTransform.headers = stream.headers;
44+
return trailingChecksumTransform;
45+
}
46+
3247
module.exports = {
3348
prepareStream,
49+
stripTrailingChecksumStream,
3450
};

‎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

‎lib/auth/streamingV4/trailingChecksumTransform.js‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
const { Transform } = require('stream');
2+
const { errors } = require('arsenal');
3+
4+
/**
5+
* This class is designed to handle the chunks sent in a streaming
6+
* v4 Auth request
7+
*/
8+
class TrailingChecksumTransform extends Transform {
9+
/**
10+
* @constructor
11+
* @param {object} log - logger object
12+
* @param {function} errCb - callback called if an error occurs
13+
*/
14+
constructor(log, errCb) {
15+
super({});
16+
this.log = log;
17+
this.errCb = errCb;
18+
this.chunkSizeBuffer = Buffer.alloc(0);
19+
this.outputBuffer = Buffer.alloc(0);
20+
this.toFlush = Buffer.alloc(0);
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 will remove the trailing checksum from the stream
27+
*
28+
* @param {Buffer} chunkInput - chunk from request body
29+
* @param {string} encoding - Data encoding
30+
* @param {function} callback - Callback(err, justDataChunk, encoding)
31+
* @return {function} executes callback with err if applicable
32+
*/
33+
_transform(chunkInput, encoding, callback) {
34+
let chunk = chunkInput;
35+
for (;;) {
36+
if (chunk.byteLength === 0) {
37+
break;
38+
}
39+
if (this.streamClosed) {
40+
return callback(null, '', encoding);
41+
}
42+
if (this.bytesToRead > 0) {
43+
if (chunk.byteLength <= this.bytesToRead) {
44+
// chunk is smaller than the advertised size
45+
// forward the whole chunk
46+
this.bytesToRead -= chunk.byteLength;
47+
this.outputBuffer = Buffer.concat([this.outputBuffer, chunk]);
48+
break;
49+
}
50+
// chunk is bigger than the advertised size
51+
this.outputBuffer = Buffer.concat([this.outputBuffer, chunk.subarray(0, this.bytesToRead)]);
52+
chunk = chunk.subarray(this.bytesToRead);
53+
this.bytesToRead = 0;
54+
continue;
55+
}
56+
const lineBreakIndex = chunk.indexOf('\r\n');
57+
if (lineBreakIndex === -1) {
58+
this.chunkSizeBuffer = Buffer.concat([this.chunkSizeBuffer, chunk]);
59+
if (this.chunkSizeBuffer.byteLength > 8) {
60+
// if bigger, the chunk would be over 5 GB
61+
// returning early to avoid a DoS by memory exhaustion
62+
return callback(errors.InvalidArgument);
63+
}
64+
return callback(null, '', encoding);
65+
}
66+
if (lineBreakIndex === 0) {
67+
chunk = chunk.subarray(lineBreakIndex + 2);
68+
continue;
69+
}
70+
this.chunkSizeBuffer = Buffer.concat([this.chunkSizeBuffer, chunk.subarray(0, lineBreakIndex)]);
71+
chunk = chunk.subarray(lineBreakIndex + 2);
72+
// chunk-size is sent in hex
73+
const dataSize = Number.parseInt(this.chunkSizeBuffer.toString(), 16);
74+
if (Number.isNaN(dataSize)) {
75+
return callback(errors.InvalidArgument);
76+
}
77+
this.chunkSizeBuffer = Buffer.alloc(0);
78+
if (dataSize === 0) {
79+
// last chunk, no more data to read, the stream is closed
80+
this.streamClosed = true;
81+
break;
82+
}
83+
this.bytesToRead = dataSize;
84+
}
85+
86+
this.toFlush = this.outputBuffer;
87+
this.outputBuffer = Buffer.alloc(0);
88+
return callback(null, this.toFlush, encoding);
89+
}
90+
}
91+
92+
module.exports = TrailingChecksumTransform;

‎tests/functional/raw-node/test/trailingChecksums.js‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
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\n0\r\nx-amz-checksum-crc64nvme:YeIDuLa7tU0=\r\n';
11+
const objDataWithoutTrailingChecksum = '0123456789abcdef';
12+
13+
const authCredentials = {
14+
accessKey: 'accessKey1',
15+
secretKey: 'verySecretKey1',
16+
};
17+
18+
const itSkipIfAWS = process.env.AWS_ON_AIR ? it.skip : it;
19+
20+
describe('trailing checksum requests:', () => {
21+
before(done => {
22+
makeS3Request({
23+
method: 'PUT',
24+
authCredentials,
25+
bucket,
26+
}, err => {
27+
assert.ifError(err);
28+
done();
29+
});
30+
});
31+
32+
after(done => {
33+
async.series([
34+
next => makeS3Request({
35+
method: 'DELETE',
36+
authCredentials,
37+
bucket,
38+
objectKey,
39+
}, next),
40+
next => makeS3Request({
41+
method: 'DELETE',
42+
authCredentials,
43+
bucket,
44+
}, next),
45+
], err => {
46+
assert.ifError(err);
47+
done();
48+
});
49+
});
50+
51+
const objDataChunkings = [
52+
[objDataWithTrailingChecksum],
53+
[objDataWithTrailingChecksum.substring(0, 1), objDataWithTrailingChecksum.substring(1)],
54+
[objDataWithTrailingChecksum.substring(0, 2), objDataWithTrailingChecksum.substring(2)],
55+
[objDataWithTrailingChecksum.substring(0, 3), objDataWithTrailingChecksum.substring(3)],
56+
[objDataWithTrailingChecksum.substring(0, 4), objDataWithTrailingChecksum.substring(4)],
57+
[objDataWithTrailingChecksum.substring(0, 10), objDataWithTrailingChecksum.substring(10)],
58+
[objDataWithTrailingChecksum.substring(0, 20), objDataWithTrailingChecksum.substring(20)],
59+
[objDataWithTrailingChecksum.substring(0, 21), objDataWithTrailingChecksum.substring(21)],
60+
[objDataWithTrailingChecksum.substring(0, 22), objDataWithTrailingChecksum.substring(22)],
61+
];
62+
63+
objDataChunkings.forEach((objDataChunks, index) => {
64+
it(`should accept unsigned trailing checksum, chunk configuration ${index + 1}`, done => {
65+
const req = new HttpRequestAuthV4(
66+
`http://localhost:8000/${bucket}/${objectKey}`,
67+
Object.assign(
68+
{
69+
method: 'PUT',
70+
headers: {
71+
'content-length': objDataWithTrailingChecksum.length,
72+
'x-amz-decoded-content-length': objDataWithoutTrailingChecksum.length,
73+
'x-amz-content-sha256': 'STREAMING-UNSIGNED-PAYLOAD-TRAILER',
74+
'x-amz-trailer': 'x-amz-checksum-crc64nvme',
75+
},
76+
},
77+
authCredentials
78+
),
79+
res => {
80+
assert.strictEqual(res.statusCode, 200);
81+
res.on('data', () => {});
82+
res.on('end', done);
83+
}
84+
);
85+
86+
req.on('error', err => {
87+
assert.ifError(err);
88+
});
89+
90+
// write the data in two chunks
91+
objDataChunks.forEach(chunk => {
92+
req.write(chunk);
93+
}
94+
);
95+
96+
req.once('drain', () => {
97+
req.end();
98+
});
99+
});
100+
101+
it('should have correct object content for unsigned trailing checksum', done => {
102+
makeS3Request({
103+
method: 'GET',
104+
authCredentials,
105+
bucket,
106+
objectKey,
107+
}, (err, res) => {
108+
assert.ifError(err);
109+
assert.strictEqual(res.statusCode, 200);
110+
// check that the object data is the input stripped of the trailing checksum
111+
assert.strictEqual(res.body, objDataWithoutTrailingChecksum);
112+
return done();
113+
});
114+
});
115+
});
116+
117+
itSkipIfAWS('should respond with BadRequest for signed trailing checksum', done => {
118+
const req = new HttpRequestAuthV4(
119+
`http://localhost:8000/${bucket}/${objectKey}`,
120+
Object.assign(
121+
{
122+
method: 'PUT',
123+
headers: {
124+
'content-length': objData.length,
125+
'x-amz-content-sha256': 'STREAMING-AWS4-HMAC-SHA256-PAYLOAD-TRAILER',
126+
'x-amz-trailer': 'x-amz-checksum-sha256',
127+
},
128+
},
129+
authCredentials
130+
),
131+
res => {
132+
assert.strictEqual(res.statusCode, 400);
133+
res.on('data', () => {});
134+
res.on('end', done);
135+
}
136+
);
137+
138+
req.on('error', err => {
139+
assert.ifError(err);
140+
});
141+
142+
req.write(objData);
143+
144+
req.once('drain', () => {
145+
req.end();
146+
});
147+
});
148+
});

0 commit comments

Comments
 (0)