Skip to content

Commit fdb29a1

Browse files
leif-scalityleif-scality
committed
CLDSRV-898: stop treating x-amz-checksum-algorithm/type as checksum digests
1 parent 2afc61e commit fdb29a1

2 files changed

Lines changed: 108 additions & 47 deletions

File tree

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

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,20 @@ const algorithms = Object.freeze({
163163
},
164164
});
165165

166+
/**
167+
* List per-algorithm `x-amz-checksum-<algo>` value headers, excluding the
168+
* `x-amz-checksum-type` and `x-amz-checksum-algorithm` configuration
169+
* headers.
170+
*
171+
* @param {object} headers - HTTP request headers (lowercased keys)
172+
* @returns {string[]} matching header names
173+
*/
174+
function listChecksumValueHeaders(headers) {
175+
return Object.keys(headers).filter(
176+
h => h.startsWith('x-amz-checksum-') && h !== 'x-amz-checksum-type' && h !== 'x-amz-checksum-algorithm',
177+
);
178+
}
179+
166180
/**
167181
* Validate the x-amz-checksum-<algo> header against a buffered body.
168182
*
@@ -172,7 +186,7 @@ const algorithms = Object.freeze({
172186
* null on success; otherwise a ChecksumError with details.
173187
*/
174188
async function validateXAmzChecksums(headers, body) {
175-
const checksumHeaders = Object.keys(headers).filter(header => header.startsWith('x-amz-checksum-'));
189+
const checksumHeaders = listChecksumValueHeaders(headers);
176190
const xAmzChecksumCnt = checksumHeaders.length;
177191
if (xAmzChecksumCnt > 1) {
178192
return { error: ChecksumError.MultipleChecksumTypes, details: { algorithms: checksumHeaders } };
@@ -256,7 +270,7 @@ function getChecksumDataFromHeaders(headers) {
256270
return null;
257271
};
258272

259-
const checksumHeaders = Object.keys(headers).filter(header => header.startsWith('x-amz-checksum-'));
273+
const checksumHeaders = listChecksumValueHeaders(headers);
260274
const xAmzChecksumCnt = checksumHeaders.length;
261275
if (xAmzChecksumCnt > 1) {
262276
return { error: ChecksumError.MultipleChecksumTypes, details: { algorithms: checksumHeaders } };
@@ -441,9 +455,7 @@ function arsenalErrorFromChecksumError(err) {
441455
function validateCompleteMultipartUploadChecksum(headers, finalChecksum) {
442456
// x-amz-checksum-type can be present in a completeMPU request so we drop it.
443457
// x-amz-checksum-algorithm is ignored by AWS in this context so we drop it.
444-
const valueHeaders = Object.keys(headers).filter(
445-
h => h.startsWith('x-amz-checksum-') && h !== 'x-amz-checksum-type' && h !== 'x-amz-checksum-algorithm',
446-
);
458+
const valueHeaders = listChecksumValueHeaders(headers);
447459

448460
if (valueHeaders.length === 0) {
449461
return null;

‎tests/unit/api/apiUtils/integrity/validateChecksums.js‎

Lines changed: 91 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
const assert = require('assert');
22
const crypto = require('crypto');
3-
const sinon = require('sinon');
43

4+
const { DummyRequestLogger } = require('../../../helpers');
55
const {
66
validateChecksumsNoChunking,
77
ChecksumError,
@@ -25,7 +25,7 @@ describe('validateChecksumsNoChunking MD5', () => {
2525
};
2626

2727
const result = await validateChecksumsNoChunking(headers, body);
28-
assert.strictEqual(result, null);
28+
assert.ifError(result);
2929
});
3030
});
3131

@@ -234,6 +234,40 @@ describe('validateChecksumsNoChunking CRC32, CRC32C, SHA1, SHA256, CRC64NVME', (
234234
assert.deepStrictEqual(result.details.algorithms, ['x-amz-checksum-sha1', 'x-amz-checksum-sha256']);
235235
});
236236

237+
it('should ignore x-amz-checksum-algorithm alongside a valid x-amz-checksum-<algo> value', async () => {
238+
const body = 'sha256 data';
239+
const headers = {
240+
'content-type': 'application/json',
241+
'x-amz-checksum-algorithm': 'SHA256',
242+
'x-amz-checksum-sha256': 'jS/UevcoKxbM33kmPFujS72ior/9/i374VmGvbTAwAc=',
243+
};
244+
const result = await validateChecksumsNoChunking(headers, body);
245+
assert.ifError(result);
246+
});
247+
248+
it('should ignore x-amz-checksum-type alongside a valid x-amz-checksum-<algo> value', async () => {
249+
const body = 'sha256 data';
250+
const headers = {
251+
'content-type': 'application/json',
252+
'x-amz-checksum-type': 'FULL_OBJECT',
253+
'x-amz-checksum-sha256': 'jS/UevcoKxbM33kmPFujS72ior/9/i374VmGvbTAwAc=',
254+
};
255+
const result = await validateChecksumsNoChunking(headers, body);
256+
assert.ifError(result);
257+
});
258+
259+
it('should ignore both x-amz-checksum-algorithm and x-amz-checksum-type when combined with a value', async () => {
260+
const body = 'sha256 data';
261+
const headers = {
262+
'content-type': 'application/json',
263+
'x-amz-checksum-algorithm': 'SHA256',
264+
'x-amz-checksum-type': 'FULL_OBJECT',
265+
'x-amz-checksum-sha256': 'jS/UevcoKxbM33kmPFujS72ior/9/i374VmGvbTAwAc=',
266+
};
267+
const result = await validateChecksumsNoChunking(headers, body);
268+
assert.ifError(result);
269+
});
270+
237271
for (const algo of algos) {
238272
it('should return error AlgoNotSupported if x-amz-sdk-checksum-algorithm algo not supported', async () => {
239273
const headers = {
@@ -291,16 +325,13 @@ describe('validateChecksumsNoChunking CRC32, CRC32C, SHA1, SHA256, CRC64NVME', (
291325
});
292326

293327
describe('validateMethodChecksumNoChunking', () => {
294-
let sandbox;
295328
let originalIntegrityChecks;
296329

297330
beforeEach(() => {
298-
sandbox = sinon.createSandbox();
299331
originalIntegrityChecks = { ...config.integrityChecks };
300332
});
301333

302334
afterEach(() => {
303-
sandbox.restore();
304335
config.integrityChecks = originalIntegrityChecks;
305336
});
306337

@@ -317,12 +348,11 @@ describe('validateMethodChecksumNoChunking', () => {
317348
'content-md5': wrongMd5,
318349
},
319350
};
320-
const log = { debug: sandbox.stub() };
351+
const log = new DummyRequestLogger();
321352

322353
const result = await validateMethodChecksumNoChunking(request, body, log);
323354

324355
assert.deepStrictEqual(result, ArsenalErrors.BadDigest, 'Expected BadDigest error');
325-
assert(log.debug.calledOnce);
326356
});
327357
});
328358
});
@@ -340,12 +370,11 @@ describe('validateMethodChecksumNoChunking', () => {
340370
'content-md5': wrongMd5,
341371
},
342372
};
343-
const log = { debug: sandbox.stub() };
373+
const log = new DummyRequestLogger();
344374

345375
const result = await validateMethodChecksumNoChunking(request, body, log);
346376

347377
assert.deepStrictEqual(result, ArsenalErrors.InvalidDigest, 'Expected BadDigest error');
348-
assert(log.debug.calledOnce);
349378
});
350379
});
351380
});
@@ -360,12 +389,11 @@ describe('validateMethodChecksumNoChunking', () => {
360389
apiMethod: method,
361390
headers: {},
362391
};
363-
const log = { debug: sandbox.stub() };
392+
const log = new DummyRequestLogger();
364393

365394
const result = await validateMethodChecksumNoChunking(request, body, log);
366395

367-
assert.strictEqual(result, null);
368-
assert(log.debug.notCalled);
396+
assert.ifError(result);
369397
});
370398
});
371399
});
@@ -383,12 +411,11 @@ describe('validateMethodChecksumNoChunking', () => {
383411
'content-md5': correctMd5,
384412
},
385413
};
386-
const log = { debug: sandbox.stub() };
414+
const log = new DummyRequestLogger();
387415

388416
const result = await validateMethodChecksumNoChunking(request, body, log);
389417

390-
assert.strictEqual(result, null);
391-
assert(log.debug.notCalled);
418+
assert.ifError(result);
392419
});
393420
});
394421
});
@@ -406,12 +433,11 @@ describe('validateMethodChecksumNoChunking', () => {
406433
'content-md5': wrongMd5,
407434
},
408435
};
409-
const log = { debug: sandbox.stub() };
436+
const log = new DummyRequestLogger();
410437

411438
const result = await validateMethodChecksumNoChunking(request, body, log);
412439

413-
assert.strictEqual(result, null);
414-
assert(log.debug.notCalled);
440+
assert.ifError(result);
415441
});
416442
});
417443
});
@@ -429,12 +455,11 @@ describe('validateMethodChecksumNoChunking', () => {
429455
'content-md5': wrongMd5,
430456
},
431457
};
432-
const log = { debug: sandbox.stub() };
458+
const log = new DummyRequestLogger();
433459

434460
const result = await validateMethodChecksumNoChunking(request, body, log);
435461

436-
assert.strictEqual(result, null);
437-
assert(log.debug.notCalled);
462+
assert.ifError(result);
438463
});
439464
});
440465

@@ -446,11 +471,11 @@ describe('validateMethodChecksumNoChunking', () => {
446471
'content-md5': 'wrongchecksum123=',
447472
},
448473
};
449-
const log = { debug: sandbox.stub() };
474+
const log = new DummyRequestLogger();
450475

451476
const result = await validateMethodChecksumNoChunking(request, body, log);
452477

453-
assert.strictEqual(result, null);
478+
assert.ifError(result);
454479
});
455480

456481
it('should return null when request apiMethod is undefined in config', async () => {
@@ -461,11 +486,11 @@ describe('validateMethodChecksumNoChunking', () => {
461486
'content-md5': 'wrongchecksum123=',
462487
},
463488
};
464-
const log = { debug: sandbox.stub() };
489+
const log = new DummyRequestLogger();
465490

466491
const result = await validateMethodChecksumNoChunking(request, body, log);
467492

468-
assert.strictEqual(result, null);
493+
assert.ifError(result);
469494
});
470495
});
471496

@@ -482,9 +507,9 @@ describe('validateMethodChecksumNoChunking', () => {
482507
apiMethod: 'completeMultipartUpload',
483508
headers: { 'content-md5': correctMd5 },
484509
};
485-
const log = { debug: sandbox.stub() };
510+
const log = new DummyRequestLogger();
486511
const result = await validateMethodChecksumNoChunking(request, body, log);
487-
assert.strictEqual(result, null);
512+
assert.ifError(result);
488513
});
489514

490515
it('should return BadDigest when Content-MD5 does not match the body', async () => {
@@ -493,10 +518,9 @@ describe('validateMethodChecksumNoChunking', () => {
493518
apiMethod: 'completeMultipartUpload',
494519
headers: { 'content-md5': '1B2M2Y8AsgTpgAmY7PhCfg==' },
495520
};
496-
const log = { debug: sandbox.stub() };
521+
const log = new DummyRequestLogger();
497522
const result = await validateMethodChecksumNoChunking(request, body, log);
498523
assert.deepStrictEqual(result, ArsenalErrors.BadDigest);
499-
assert(log.debug.calledOnce);
500524
});
501525

502526
it('should return InvalidDigest when Content-MD5 is malformed', async () => {
@@ -505,7 +529,7 @@ describe('validateMethodChecksumNoChunking', () => {
505529
apiMethod: 'completeMultipartUpload',
506530
headers: { 'content-md5': 'wrongchecksum123=' },
507531
};
508-
const log = { debug: sandbox.stub() };
532+
const log = new DummyRequestLogger();
509533
const result = await validateMethodChecksumNoChunking(request, body, log);
510534
assert.deepStrictEqual(result, ArsenalErrors.InvalidDigest);
511535
});
@@ -516,9 +540,9 @@ describe('validateMethodChecksumNoChunking', () => {
516540
apiMethod: 'completeMultipartUpload',
517541
headers: {},
518542
};
519-
const log = { debug: sandbox.stub() };
543+
const log = new DummyRequestLogger();
520544
const result = await validateMethodChecksumNoChunking(request, body, log);
521-
assert.strictEqual(result, null);
545+
assert.ifError(result);
522546
});
523547

524548
it('should NOT validate x-amz-checksum-* as a body digest (final-object semantics)', async () => {
@@ -531,9 +555,9 @@ describe('validateMethodChecksumNoChunking', () => {
531555
apiMethod: 'completeMultipartUpload',
532556
headers: { 'x-amz-checksum-sha256': 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=' },
533557
};
534-
const log = { debug: sandbox.stub() };
558+
const log = new DummyRequestLogger();
535559
const result = await validateMethodChecksumNoChunking(request, body, log);
536-
assert.strictEqual(result, null);
560+
assert.ifError(result);
537561
});
538562
});
539563
});
@@ -550,12 +574,12 @@ describe('getChecksumDataFromHeaders', () => {
550574

551575
it('should return null when no headers', () => {
552576
const result = getChecksumDataFromHeaders({});
553-
assert.strictEqual(result, null);
577+
assert.ifError(result);
554578
});
555579

556580
it('should return null when no checksum headers, no trailer, no sdk algo', () => {
557581
const result = getChecksumDataFromHeaders({ 'content-type': 'application/octet-stream' });
558-
assert.strictEqual(result, null);
582+
assert.ifError(result);
559583
});
560584

561585
for (const [algo, digest] of Object.entries(validDigests)) {
@@ -591,6 +615,31 @@ describe('getChecksumDataFromHeaders', () => {
591615
assert.strictEqual(result.error, ChecksumError.MultipleChecksumTypes);
592616
});
593617

618+
it('should ignore x-amz-checksum-algorithm alongside a valid x-amz-checksum-<algo> value', () => {
619+
const result = getChecksumDataFromHeaders({
620+
'x-amz-checksum-algorithm': 'SHA256',
621+
'x-amz-checksum-sha256': validDigests.sha256,
622+
});
623+
assert.deepStrictEqual(result, { algorithm: 'sha256', isTrailer: false, expected: validDigests.sha256 });
624+
});
625+
626+
it('should ignore x-amz-checksum-type alongside a valid x-amz-checksum-<algo> value', () => {
627+
const result = getChecksumDataFromHeaders({
628+
'x-amz-checksum-type': 'FULL_OBJECT',
629+
'x-amz-checksum-sha256': validDigests.sha256,
630+
});
631+
assert.deepStrictEqual(result, { algorithm: 'sha256', isTrailer: false, expected: validDigests.sha256 });
632+
});
633+
634+
it('should ignore both x-amz-checksum-algorithm and x-amz-checksum-type when combined with a value', () => {
635+
const result = getChecksumDataFromHeaders({
636+
'x-amz-checksum-algorithm': 'SHA256',
637+
'x-amz-checksum-type': 'FULL_OBJECT',
638+
'x-amz-checksum-sha256': validDigests.sha256,
639+
});
640+
assert.deepStrictEqual(result, { algorithm: 'sha256', isTrailer: false, expected: validDigests.sha256 });
641+
});
642+
594643
it('should return MissingCorresponding when x-amz-sdk-checksum-algorithm has no x-amz-checksum- or trailer', () => {
595644
const result = getChecksumDataFromHeaders({ 'x-amz-sdk-checksum-algorithm': 'crc32' });
596645
assert.strictEqual(result.error, ChecksumError.MissingCorresponding);
@@ -683,7 +732,7 @@ describe('getChecksumDataFromHeaders', () => {
683732
describe('arsenalErrorFromChecksumError', () => {
684733
it('should return null for MissingChecksum', () => {
685734
const result = arsenalErrorFromChecksumError({ error: ChecksumError.MissingChecksum, details: null });
686-
assert.strictEqual(result, null);
735+
assert.ifError(result);
687736
});
688737

689738
it('should return BadDigest mentioning CRC32 for XAmzMismatch with crc32', () => {
@@ -1011,7 +1060,7 @@ describe('validateCompleteMultipartUploadChecksum', () => {
10111060
{ host: 'example.com' },
10121061
{ algorithm: 'sha256', type: 'COMPOSITE', value: `${SHA256_A}-3` },
10131062
);
1014-
assert.strictEqual(err, null);
1063+
assert.ifError(err);
10151064
});
10161065

10171066
it('should ignore x-amz-checksum-type and x-amz-checksum-algorithm headers', () => {
@@ -1022,15 +1071,15 @@ describe('validateCompleteMultipartUploadChecksum', () => {
10221071
},
10231072
{ algorithm: 'sha256', type: 'COMPOSITE', value: `${SHA256_A}-3` },
10241073
);
1025-
assert.strictEqual(err, null);
1074+
assert.ifError(err);
10261075
});
10271076

10281077
it('should return null when COMPOSITE header value matches (digest-N form)', () => {
10291078
const err = validateCompleteMultipartUploadChecksum(
10301079
{ 'x-amz-checksum-sha256': `${SHA256_A}-3` },
10311080
{ algorithm: 'sha256', type: 'COMPOSITE', value: `${SHA256_A}-3` },
10321081
);
1033-
assert.strictEqual(err, null);
1082+
assert.ifError(err);
10341083
});
10351084

10361085
it('should return null when FULL_OBJECT header value matches (no suffix)', () => {
@@ -1041,7 +1090,7 @@ describe('validateCompleteMultipartUploadChecksum', () => {
10411090
{ 'x-amz-checksum-crc32': CRC32_A },
10421091
{ algorithm: 'crc32', type: 'FULL_OBJECT', value: CRC32_A },
10431092
);
1044-
assert.strictEqual(err, null);
1093+
assert.ifError(err);
10451094
});
10461095

10471096
it('should return XAmzMismatch when header value differs', () => {
@@ -1075,7 +1124,7 @@ describe('validateCompleteMultipartUploadChecksum', () => {
10751124

10761125
it('should return null when finalChecksum is null and no header present', () => {
10771126
const err = validateCompleteMultipartUploadChecksum({ host: 'example.com' }, null);
1078-
assert.strictEqual(err, null);
1127+
assert.ifError(err);
10791128
});
10801129

10811130
it('should return MultipleChecksumTypes when multiple x-amz-checksum-* headers are sent', () => {

0 commit comments

Comments
 (0)