diff --git a/lib/auth/v4/createCanonicalRequest.js b/lib/auth/v4/createCanonicalRequest.js index 4198abf3f..41f923c29 100644 --- a/lib/auth/v4/createCanonicalRequest.js +++ b/lib/auth/v4/createCanonicalRequest.js @@ -43,9 +43,7 @@ function createCanonicalRequest(params) { const signedHeaders = signedHeadersList.join(';'); // canonical headers - const canonicalHeadersList = signedHeadersList.filter(signedHeader => - pHeaders[signedHeader] - ).map(signedHeader => + const canonicalHeadersList = signedHeadersList.map(signedHeader => `${signedHeader}:${pHeaders[signedHeader]}\n` ); diff --git a/lib/auth/v4/headerAuthCheck.js b/lib/auth/v4/headerAuthCheck.js index 79ab29105..304dd88e9 100644 --- a/lib/auth/v4/headerAuthCheck.js +++ b/lib/auth/v4/headerAuthCheck.js @@ -7,6 +7,8 @@ const checkTimeSkew = require('./timeUtils').checkTimeSkew; const convertUTCtoISO8601 = require('./timeUtils').convertUTCtoISO8601; const extractAuthItems = require('./validateInputs').extractAuthItems; const validateCredentials = require('./validateInputs').validateCredentials; +const areSignedHeadersComplete = + require('./validateInputs').areSignedHeadersComplete; let vault = require('../vault'); @@ -58,6 +60,11 @@ headerAuthCheck.check = (request, log, callback, awsService) => { const credentialsArr = authHeaderItems.credentialsArr; const signedHeaders = authHeaderItems.signedHeaders; + if (!areSignedHeadersComplete(signedHeaders, request.headers)) { + log.debug('signedHeaders are incomplete', { signedHeaders }); + return callback(errors.AccessDenied); + } + let timestamp; // check request timestamp if (request.headers['x-amz-date']) { diff --git a/lib/auth/v4/queryAuthCheck.js b/lib/auth/v4/queryAuthCheck.js index 243725432..d7779839b 100644 --- a/lib/auth/v4/queryAuthCheck.js +++ b/lib/auth/v4/queryAuthCheck.js @@ -6,6 +6,8 @@ const constructStringToSign = require('./constructStringToSign'); const checkTimeSkew = require('./timeUtils').checkTimeSkew; const validateCredentials = require('./validateInputs').validateCredentials; const extractQueryParams = require('./validateInputs').extractQueryParams; +const areSignedHeadersComplete = + require('./validateInputs').areSignedHeadersComplete; let vault = require('../vault'); @@ -35,6 +37,11 @@ queryAuthCheck.check = (request, log, callback) => { const expiry = authParams.expiry; const credential = authParams.credential; + if (!areSignedHeadersComplete(signedHeaders, request.headers)) { + log.debug('signedHeaders are incomplete', { signedHeaders }); + return callback(errors.AccessDenied); + } + if (!validateCredentials(credential, timestamp, log)) { log.warn('credential param format incorrect', { credential }); return callback(errors.InvalidArgument); diff --git a/lib/auth/v4/validateInputs.js b/lib/auth/v4/validateInputs.js index a01ffdd5e..7cadce4c1 100644 --- a/lib/auth/v4/validateInputs.js +++ b/lib/auth/v4/validateInputs.js @@ -76,6 +76,7 @@ function extractQueryParams(queryObj, log) { return authParams; } + const signature = queryObj['X-Amz-Signature']; if (signature && signature.length === 64) { authParams.signatureFromRequest = signature; @@ -156,4 +157,28 @@ function extractAuthItems(authHeader, log) { return authItems; } -module.exports = { validateCredentials, extractQueryParams, extractAuthItems }; +/** + * Checks whether the signed headers include the host header + * and all x-amz- and x-scal- headers in request + * @param {string} signedHeaders - signed headers sent with request + * @param {object} allHeaders - request.headers + * @return {boolean} true if all x-amz-headers included and false if not + */ +function areSignedHeadersComplete(signedHeaders, allHeaders) { + const signedHeadersList = signedHeaders.split(';'); + if (signedHeadersList.indexOf('host') === -1) { + return false; + } + const headers = Object.keys(allHeaders); + for (let i = 0; i < headers.length; i++) { + if ((headers[i].startsWith('x-amz-') + || headers[i].startsWith('x-scal-')) + && signedHeadersList.indexOf(headers[i]) === -1) { + return false; + } + } + return true; +} + +module.exports = { validateCredentials, extractQueryParams, + areSignedHeadersComplete, extractAuthItems }; diff --git a/tests/unit/auth/v4/headerAuthCheck.js b/tests/unit/auth/v4/headerAuthCheck.js index 6f34e23f9..7e1d19756 100644 --- a/tests/unit/auth/v4/headerAuthCheck.js +++ b/tests/unit/auth/v4/headerAuthCheck.js @@ -56,6 +56,40 @@ describe('v4 headerAuthCheck', () => { }); }); + it('should return error if host is not included as signed header', done => { + const alteredRequest = createAlteredRequest({ + authorization: 'AWS4-HMAC-SHA256 Credential=accessKey1/20160208' + + '/us-east-1/s3/aws4_request, ' + + 'SignedHeaders=x-amz-content-sha256;' + + 'x-amz-date, ' + + 'Signature=abed924c06abf8772c670064d22eacd6ccb85c06befa15f' + + '4a789b0bae19307bc' }, 'headers', request, headers); + headerAuthCheck(alteredRequest, log, err => { + assert.deepStrictEqual(err, errors.AccessDenied); + done(); + }); + }); + + it('should return error if an x-amz header is not included as signed ' + + 'header but is in request', done => { + const alteredRequest = createAlteredRequest({ + 'x-amz-acl': 'public' }, 'headers', request, headers); + headerAuthCheck(alteredRequest, log, err => { + assert.deepStrictEqual(err, errors.AccessDenied); + done(); + }); + }); + + it('should return error if an x-scal header is not included as signed ' + + 'header but is in request', done => { + const alteredRequest = createAlteredRequest({ + 'x-scal-encryption': 'true' }, 'headers', request, headers); + headerAuthCheck(alteredRequest, log, err => { + assert.deepStrictEqual(err, errors.AccessDenied); + done(); + }); + }); + it('should return error if missing credentials', done => { const alteredRequest = createAlteredRequest({ authorization: 'AWS4-HMAC-SHA256 SignedHeaders=host;' + diff --git a/tests/unit/auth/v4/queryAuthCheck.js b/tests/unit/auth/v4/queryAuthCheck.js index b0951073d..93c8455fc 100644 --- a/tests/unit/auth/v4/queryAuthCheck.js +++ b/tests/unit/auth/v4/queryAuthCheck.js @@ -25,10 +25,13 @@ const query = { '7723157d8148ad5888b3aee1133784eb5aec08b', 'X-Amz-SignedHeaders': 'host', }; +const headers = { + host, +}; const request = { method, path, - headers: { host }, + headers, query, }; @@ -103,6 +106,35 @@ describe('v4 queryAuthCheck', () => { }); }); + it('should return error if host is not included as signed header', done => { + const alteredRequest = createAlteredRequest({ 'X-Amz-SignedHeaders': + 'none' }, 'query', request, query); + queryAuthCheck(alteredRequest, log, err => { + assert.deepStrictEqual(err, errors.AccessDenied); + done(); + }); + }); + + it('should return error if an x-amz header is not included as signed ' + + 'header but is in request', done => { + const alteredRequest = createAlteredRequest({ + 'x-amz-acl': 'public' }, 'headers', request, headers); + queryAuthCheck(alteredRequest, log, err => { + assert.deepStrictEqual(err, errors.AccessDenied); + done(); + }); + }); + + it('should return error if an x-scal header is not included as signed ' + + 'header but is in request', done => { + const alteredRequest = createAlteredRequest({ + 'x-scal-encryption': 'true' }, 'headers', request, headers); + queryAuthCheck(alteredRequest, log, err => { + assert.deepStrictEqual(err, errors.AccessDenied); + done(); + }); + }); + it('should return error if undefined X-Amz-Date param', done => { const alteredRequest = createAlteredRequest({ 'X-Amz-Date': undefined }, 'query', request, query);