Skip to content

Commit b10a2d6

Browse files
committed
CLDSRV-729: don't require content-md5 header in put-bucket-cors and delete-objects
(cherry picked from commit e529da9)
1 parent e1f9dd1 commit b10a2d6

4 files changed

Lines changed: 162 additions & 11 deletions

File tree

lib/api/bucketPutCors.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,12 @@ function bucketPutCors(authInfo, request, log, callback) {
3131
return callback(errors.MissingRequestBodyError);
3232
}
3333

34-
const md5 = crypto.createHash('md5')
35-
.update(request.post, 'utf8').digest('base64');
36-
if (md5 !== request.headers['content-md5']) {
37-
log.debug('bad md5 digest', { error: errors.BadDigest });
38-
return callback(errors.BadDigest);
34+
if (request.headers['content-md5']) {
35+
const md5 = crypto.createHash('md5').update(request.post, 'utf8').digest('base64');
36+
if (md5 !== request.headers['content-md5']) {
37+
log.debug('bad md5 digest', { error: errors.BadDigest });
38+
return callback(errors.BadDigest);
39+
}
3940
}
4041

4142
if (parseInt(request.headers['content-length'], 10) > 65536) {

lib/api/multiObjectDelete.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -366,10 +366,12 @@ function multiObjectDelete(authInfo, request, log, callback) {
366366
if (!request.post) {
367367
return callback(errors.MissingRequestBodyError);
368368
}
369-
const md5 = crypto.createHash('md5')
370-
.update(request.post, 'utf8').digest('base64');
371-
if (md5 !== request.headers['content-md5']) {
372-
return callback(errors.BadDigest);
369+
370+
if (request.headers['content-md5']) {
371+
const md5 = crypto.createHash('md5').update(request.post, 'utf8').digest('base64');
372+
if (md5 !== request.headers['content-md5']) {
373+
return callback(errors.BadDigest);
374+
}
373375
}
374376

375377
const bucketName = request.bucketName;

tests/unit/api/bucketPutCors.js

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ describe('putBucketCORS API', () => {
5555
.createBucketCorsRequest('PUT', bucketName);
5656
bucketPutCors(authInfo, testBucketPutCorsRequest, log, err => {
5757
if (err) {
58-
process.stdout.write(`Err putting website config ${err}`);
58+
process.stdout.write(`Err putting bucket cors ${err}`);
5959
return done(err);
6060
}
6161
return metadata.getBucket(bucketName, log, (err, bucket) => {
@@ -70,11 +70,33 @@ describe('putBucketCORS API', () => {
7070
});
7171
});
7272

73-
it('should return BadDigest if md5 is omitted', done => {
73+
it('should accept request if md5 is omitted', done => {
7474
const corsUtil = new CorsConfigTester();
7575
const testBucketPutCorsRequest = corsUtil
7676
.createBucketCorsRequest('PUT', bucketName);
7777
testBucketPutCorsRequest.headers['content-md5'] = undefined;
78+
bucketPutCors(authInfo, testBucketPutCorsRequest, log, err => {
79+
if (err) {
80+
process.stdout.write(`Err putting bucket cors ${err}`);
81+
return done(err);
82+
}
83+
return metadata.getBucket(bucketName, log, (err, bucket) => {
84+
if (err) {
85+
process.stdout.write(`Err retrieving bucket MD ${err}`);
86+
return done(err);
87+
}
88+
const uploadedCors = bucket.getCors();
89+
assert.deepStrictEqual(uploadedCors, corsUtil.getCors());
90+
return done();
91+
});
92+
});
93+
});
94+
95+
it('should reject request if md5 is mismatch', done => {
96+
const corsUtil = new CorsConfigTester();
97+
const testBucketPutCorsRequest = corsUtil
98+
.createBucketCorsRequest('PUT', bucketName);
99+
testBucketPutCorsRequest.headers['content-md5'] = 'wrong md5';
78100
_testPutBucketCors(authInfo, testBucketPutCorsRequest,
79101
log, 'BadDigest', done);
80102
});

tests/unit/api/multiObjectDelete.js

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
const assert = require('assert');
22
const { errors } = require('arsenal');
3+
const sinon = require('sinon');
4+
const crypto = require('crypto');
5+
6+
const metadataWrapper = require('../../../lib/metadata/wrapper');
7+
const multiObjectDelete = require('../../../lib/api/multiObjectDelete');
38

49
const { getObjMetadataAndDelete }
510
= require('../../../lib/api/multiObjectDelete');
@@ -19,6 +24,8 @@ const postBody = Buffer.from('I am a body', 'utf8');
1924
const contentLength = 2 * postBody.length;
2025
const objectKey1 = 'objectName1';
2126
const objectKey2 = 'objectName2';
27+
const { BucketInfo } = require('arsenal/build/lib/models');
28+
2229
const testBucketPutRequest = new DummyRequest({
2330
bucketName,
2431
namespace,
@@ -185,3 +192,122 @@ describe('getObjMetadataAndDelete function for multiObjectDelete', () => {
185192
});
186193
});
187194
});
195+
196+
describe('multiObjectDelete function', () => {
197+
afterEach(() => {
198+
sinon.restore();
199+
});
200+
201+
it('should not authorize the bucket and initial IAM authorization results', done => {
202+
const post = '<Delete><Object><Key>objectname</Key></Object></Delete>';
203+
const request = new DummyRequest({
204+
bucketName: 'bucketname',
205+
objectKey: 'objectname',
206+
parsedHost: 'localhost',
207+
headers: {
208+
'content-md5': crypto.createHash('md5').update(post, 'utf8').digest('base64'),
209+
},
210+
post,
211+
socket: {
212+
remoteAddress: '127.0.0.1',
213+
},
214+
url: '/bucketname',
215+
});
216+
const authInfo = makeAuthInfo('123456');
217+
218+
sinon.stub(metadataWrapper, 'getBucket').callsFake((bucketName, log, cb) =>
219+
cb(null, new BucketInfo(
220+
'bucketname',
221+
'123456',
222+
'accountA',
223+
new Date().toISOString(),
224+
15,
225+
)));
226+
227+
multiObjectDelete.multiObjectDelete(authInfo, request, log, (err, res) => {
228+
// Expected result is an access denied on the object, and no error, as the API was authorized
229+
assert.strictEqual(err, null);
230+
assert.strictEqual(
231+
res.includes('<Error><Key>objectname</Key><Code>AccessDenied</Code>'),
232+
true
233+
);
234+
done();
235+
});
236+
});
237+
238+
it('should accept request when content-md5 header is missing', done => {
239+
const post = '<Delete><Object><Key>objectname</Key></Object></Delete>';
240+
const testObjectKey = 'objectname';
241+
const testBucketName = 'test-bucket';
242+
const request = new DummyRequest({
243+
bucketName: testBucketName,
244+
objectKey: testObjectKey,
245+
parsedHost: 'localhost',
246+
headers: {
247+
// No content-md5 header
248+
},
249+
post,
250+
socket: {
251+
remoteAddress: '127.0.0.1',
252+
},
253+
url: `/${testBucketName}`,
254+
});
255+
// Use the same canonicalID for both authInfo and bucket owner to avoid AccessDenied
256+
const testAuthInfo = makeAuthInfo(canonicalID);
257+
258+
// Create bucket with proper ownership
259+
const testBucketRequest = new DummyRequest({
260+
bucketName: testBucketName,
261+
namespace,
262+
headers: {},
263+
url: `/${testBucketName}`,
264+
});
265+
// Create object to delete
266+
const testObjectRequest = new DummyRequest({
267+
bucketName: testBucketName,
268+
namespace,
269+
objectKey: testObjectKey,
270+
headers: {},
271+
url: `/${testBucketName}/${testObjectKey}`,
272+
}, postBody);
273+
274+
bucketPut(testAuthInfo, testBucketRequest, log, () => {
275+
objectPut(testAuthInfo, testObjectRequest, undefined, log, () => {
276+
multiObjectDelete.multiObjectDelete(testAuthInfo, request, log, (err, res) => {
277+
// Request should succeed even without content-md5 header
278+
assert.strictEqual(err, null);
279+
assert.strictEqual(typeof res, 'string');
280+
// Should contain successful deletion response
281+
assert.strictEqual(res.includes('<Deleted><Key>objectname</Key></Deleted>'), true);
282+
done();
283+
});
284+
});
285+
});
286+
});
287+
288+
it('should reject request with BadDigest error when content-md5 header mismatches', done => {
289+
const post = '<Delete><Object><Key>objectname</Key></Object></Delete>';
290+
const incorrectMd5 = 'incorrectMd5Hash';
291+
const request = new DummyRequest({
292+
bucketName: 'bucketname',
293+
objectKey: 'objectname',
294+
parsedHost: 'localhost',
295+
headers: {
296+
'content-md5': incorrectMd5,
297+
},
298+
post,
299+
socket: {
300+
remoteAddress: '127.0.0.1',
301+
},
302+
url: '/bucketname',
303+
});
304+
const authInfo = makeAuthInfo('123456');
305+
306+
multiObjectDelete.multiObjectDelete(authInfo, request, log, (err, res) => {
307+
// Should return BadDigest error for mismatched content-md5
308+
assert.strictEqual(err.is.BadDigest, true);
309+
assert.strictEqual(res, undefined);
310+
done();
311+
});
312+
});
313+
});

0 commit comments

Comments
 (0)