Skip to content

Commit 52c366f

Browse files
leif-scalityleif-scality
committed
CLDSRV-848: check x-amz-checksum-[crc64nvme, crc32, crc32C, sha1, sha256] headers
1 parent 5889490 commit 52c366f

File tree

5 files changed

+360
-63
lines changed

5 files changed

+360
-63
lines changed

‎lib/Config.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,7 @@ function parseIntegrityChecks(config) {
556556
'bucketPutReplication': true,
557557
'bucketPutVersioning': true,
558558
'bucketPutWebsite': true,
559+
'bucketPutLogging': true,
559560
'multiObjectDelete': true,
560561
'objectPutACL': true,
561562
'objectPutLegalHold': true,

‎lib/api/api.js‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -300,15 +300,19 @@ const api = {
300300
}
301301

302302
const buff = Buffer.concat(post, bodyLength);
303+
return validateMethodChecksumNoChunking(request, buff, log)
304+
.then(error => {
305+
if (error) {
306+
return next(error);
307+
}
303308

304-
const err = validateMethodChecksumNoChunking(request, buff, log);
305-
if (err) {
306-
return next(err);
307-
}
308-
309-
// Convert array of post buffers into one string
310-
request.post = buff.toString();
311-
return next(null, userInfo, authorizationResults, streamingV4Params, infos);
309+
// Convert array of post buffers into one string
310+
request.post = buff.toString();
311+
return next(null, userInfo, authorizationResults, streamingV4Params, infos);
312+
})
313+
.catch(error => {
314+
next(error);
315+
});
312316
});
313317
return undefined;
314318
},

‎lib/api/apiUtils/integrity/validateChecksums.js‎

Lines changed: 145 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,171 @@
11
const crypto = require('crypto');
2+
const { Crc32 } = require('@aws-crypto/crc32');
3+
const { Crc32c } = require('@aws-crypto/crc32c');
4+
const { CrtCrc64Nvme } = require('@aws-sdk/crc64-nvme-crt');
25
const { errors: ArsenalErrors } = require('arsenal');
36
const { config } = require('../../../Config');
47

58
const ChecksumError = Object.freeze({
69
MD5Mismatch: 'MD5Mismatch',
10+
XAmzMismatch: 'XAmzMismatch',
711
MissingChecksum: 'MissingChecksum',
12+
AlgoNotSupported: 'AlgoNotSupported',
13+
MultipleChecksumTypes: 'MultipleChecksumTypes',
14+
MissingCorresponding: 'MissingCorresponding'
815
});
916

17+
// TEMP
18+
// https://github.com/aws/aws-sdk-js-v3/issues/6744
19+
// https://stackoverflow.com/questions/77663519/
20+
// does-aws-s3-allow-specifying-multiple-checksum-values-crc32-crc32c-sha1-and-s
21+
// Expecting a single x-amz-checksum- header. Multiple checksum Types are not allowed.
22+
// XAmzContentChecksumMismatch: The provided 'x-amz-checksum' header does not match what was computed.
23+
24+
// TODO:
25+
// x-amz-checksum-algorithm x2 different
26+
// x-amz-checksum-algorithm x2 equal
27+
// x-amz-checksum-algorithm x2 + x-amz-checksum- valid x2
28+
// x-amz-checksum-algorithm x2 + x-amz-checksum- invalid x2
29+
// x-amz-checksum-algorithm x2 + x-amz-checksum- mismatch x2
30+
31+
// What to do about the security vuln package?
32+
// Should we push only to 9.3???
33+
34+
const algorithms = {
35+
'crc64nvme': async data => {
36+
const input = Buffer.isBuffer(data) ? data : Buffer.from(data);
37+
const crc = new CrtCrc64Nvme();
38+
crc.update(input);
39+
const result = await crc.digest();
40+
return Buffer.from(result).toString('base64');
41+
},
42+
'crc32': data => {
43+
const input = Buffer.isBuffer(data) ? data : Buffer.from(data);
44+
return uint32ToBase64(new Crc32().update(input).digest() >>> 0);
45+
},
46+
'crc32c': data => {
47+
const input = Buffer.isBuffer(data) ? data : Buffer.from(data);
48+
return uint32ToBase64(new Crc32c().update(input).digest() >>> 0);
49+
},
50+
'sha1': data => {
51+
const input = Buffer.isBuffer(data) ? data : Buffer.from(data);
52+
return crypto.createHash('sha1').update(input).digest('base64');
53+
},
54+
'sha256': data => {
55+
const input = Buffer.isBuffer(data) ? data : Buffer.from(data);
56+
return crypto.createHash('sha256').update(input).digest('base64');
57+
}
58+
};
59+
60+
function uint32ToBase64(num) {
61+
const buf = Buffer.alloc(4);
62+
buf.writeUInt32BE(num, 0);
63+
return buf.toString('base64');
64+
}
65+
66+
async function validateXAmzChecksums(headers, body) {
67+
const checksumHeaders = Object.keys(headers).filter(header => header.startsWith('x-amz-checksum-'));
68+
const xAmzCheckumCnt = checksumHeaders.length;
69+
if (xAmzCheckumCnt > 1) {
70+
return { error: ChecksumError.MultipleChecksumTypes, details: { algorithms: checksumHeaders } };
71+
}
72+
73+
if ('x-amz-sdk-checksum-algorithm' in headers) {
74+
const algo = headers['x-amz-sdk-checksum-algorithm'];
75+
if (typeof algo !== 'string') {
76+
return { error: ChecksumError.AlgoNotSupported, details: { algorithm: algo } };
77+
}
78+
79+
const lowerAlgo = algo.toLowerCase();
80+
if (lowerAlgo in algorithms === false) {
81+
return { error: ChecksumError.AlgoNotSupported, details: { algorithm: algo } };
82+
}
83+
84+
if (`x-amz-checksum-${lowerAlgo}` in headers === false) {
85+
return { error: ChecksumError.MissingCorresponding, details: { expected: `x-amz-checksum-${lowerAlgo}` } };
86+
}
87+
88+
const expected = headers[`x-amz-checksum-${lowerAlgo}`];
89+
const calculated = await algorithms[lowerAlgo](body);
90+
// console.log('EXPECTED:', expected, calculated);
91+
// console.log('==========', body, calculated, expected, expected !== calculated)
92+
if (expected !== calculated) {
93+
return { error: ChecksumError.XAmzMismatch, details: { calculated, expected } };
94+
}
95+
96+
return null;
97+
}
98+
99+
if (xAmzCheckumCnt === 0) {
100+
return { error: ChecksumError.MissingChecksum, details: null };
101+
}
102+
103+
// No x-amz-sdk-checksum-algorithm we expect one x-amz-checksum-[crc64nvme, crc32, crc32C, sha1, sha256].
104+
const algo = checksumHeaders[0].split('-')[3];
105+
if (typeof algo !== 'string') {
106+
return { error: ChecksumError.AlgoNotSupported, details: { algorithm: algo } };
107+
}
108+
109+
const lowerAlgo = algo.toLowerCase();
110+
if (lowerAlgo in algorithms === false) {
111+
return { error: ChecksumError.AlgoNotSupported, details: { algorithm: algo } };;
112+
}
113+
114+
const expected = headers[`x-amz-checksum-${lowerAlgo}`];
115+
const calculated = await algorithms[lowerAlgo](body);
116+
// console.log('==========', body, calculated, expected, expected !== calculated )
117+
if (expected !== calculated) {
118+
return { error: ChecksumError.XAmzMismatch, details: { calculated, expected } };
119+
}
120+
121+
return null;
122+
}
123+
10124
/**
11125
* validateChecksumsNoChunking - Validate the checksums of a request.
12126
* @param {object} headers - http headers
13127
* @param {Buffer} body - http request body
14128
* @return {object} - error
15129
*/
16-
function validateChecksumsNoChunking(headers, body) {
17-
if (headers && 'content-md5' in headers) {
130+
async function validateChecksumsNoChunking(headers, body) {
131+
if (!headers) {
132+
return { error: ChecksumError.MissingChecksum, details: null };
133+
}
134+
135+
let md5Present = false;
136+
if ('content-md5' in headers) {
18137
const md5 = crypto.createHash('md5').update(body).digest('base64');
19138
if (md5 !== headers['content-md5']) {
20139
return { error: ChecksumError.MD5Mismatch, details: { calculated: md5, expected: headers['content-md5'] } };
21140
}
22141

142+
md5Present = true;
143+
}
144+
145+
const err = await validateXAmzChecksums(headers, body);
146+
if (err && err.error === ChecksumError.MissingChecksum && md5Present) {
147+
// Don't return MissingChecksum if MD5 is present.
23148
return null;
24149
}
25150

26-
return { error: ChecksumError.MissingChecksum, details: null };
151+
return err;
27152
}
28153

29-
function defaultValidationFunc(request, body, log) {
30-
const err = validateChecksumsNoChunking(request.headers, body);
154+
// async function defaultValidationFunc2(request, body, log) { // Rename
155+
// const err = await validateChecksumsNoChunking(request.headers, body);
156+
// if (err) {
157+
// log.debug('failed checksum validation', { method: request.apiMethod, error: err });
158+
// return ArsenalErrors.BadDigest; // FIXME: InvalidDigest vs BadDigest
159+
// }
160+
161+
// return null;
162+
// }
163+
164+
async function defaultValidationFunc(request, body, log) { // Rename
165+
const err = await validateChecksumsNoChunking(request.headers, body);
31166
if (err && err.error !== ChecksumError.MissingChecksum) {
32167
log.debug('failed checksum validation', { method: request.apiMethod }, err);
33-
return ArsenalErrors.BadDigest;
168+
return ArsenalErrors.BadDigest; // FIXME: InvalidDigest vs BadDigest
34169
}
35170

36171
return null;
@@ -47,6 +182,7 @@ const methodValidationFunc = Object.freeze({
47182
'bucketPutReplication': defaultValidationFunc,
48183
'bucketPutVersioning': defaultValidationFunc,
49184
'bucketPutWebsite': defaultValidationFunc,
185+
'bucketPutLogging': defaultValidationFunc,
50186
// TODO: DeleteObjects requires a checksum. Should return an error if ChecksumError.MissingChecksum.
51187
'multiObjectDelete': defaultValidationFunc,
52188
'objectPutACL': defaultValidationFunc,
@@ -62,14 +198,13 @@ const methodValidationFunc = Object.freeze({
62198
* @param {object} log - logger
63199
* @return {object} - error
64200
*/
65-
function validateMethodChecksumNoChunking(request, body, log) {
201+
async function validateMethodChecksumNoChunking(request, body, log) {
66202
if (config.integrityChecks[request.apiMethod]) {
67203
const validationFunc = methodValidationFunc[request.apiMethod];
68204
if (!validationFunc) {
69-
return null;
205+
return null; //await defaultValidationFunc2(request, body, log);
70206
}
71-
72-
return validationFunc(request, body, log);
207+
return await validationFunc(request, body, log);
73208
}
74209

75210
return null;

‎tests/unit/Config.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -933,6 +933,7 @@ describe('Config', () => {
933933
'bucketPutReplication': false,
934934
'bucketPutVersioning': false,
935935
'bucketPutWebsite': false,
936+
'bucketPutLogging': false,
936937
'multiObjectDelete': false,
937938
'objectPutACL': false,
938939
'objectPutLegalHold': false,

0 commit comments

Comments
 (0)