Merge pull request #125 from scality/fix/checkHeadersRel

Fix/check headers rel
scality · Jul 29, 2016 · 78bef3c · 78bef3c
2 parents 1ffbbab + a205df5
commit 78bef3c
Show file tree

Hide file tree

Showing 6 changed files with 108 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);