CLDSRV-1012: Backport KMS error and socket leak on DataWrapper

lib/api/objectGet.js

@@ -1,5 +1,6 @@
 const { errors, s3middleware } = require('arsenal');
 const { parseRange } = require('arsenal').network.http.utils;
+const async = require('async');
 
 const { data } = require('../data/wrapper');
 
@@ -12,6 +13,7 @@ const setPartRanges = require('./apiUtils/object/setPartRanges');
 const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils');
 const { getPartCountFromMd5 } = require('./apiUtils/object/partInfo');
 const { setExpirationHeaders } = require('./apiUtils/object/expirationHeaders');
+const kms = require('../kms/wrapper');
 
 const validateHeaders = s3middleware.validateConditionalHeaders;
 
@@ -211,24 +213,57 @@ function objectGet(authInfo, request, returnTagCount, log, callback) {
                 dataLocator = setPartRanges(dataLocator, byteRange);
             }
         }
-        return data.head(dataLocator, log, err => {
-            if (err) {
-                log.error('error from external backend checking for ' +
-                'object existence', { error: err });
-                return callback(err);
+        // Check KMS Key access and usability before checking data
+        // diff with AWS: for empty object (no dataLocator) KMS not checked
+        return async.each(dataLocator || [],
+            (objectGetInfo, next) => {
+                if (!objectGetInfo.cipheredDataKey) {
+                    return next();
+                }
+                const serverSideEncryption = {
+                    cryptoScheme: objectGetInfo.cryptoScheme,
+                    masterKeyId: objectGetInfo.masterKeyId,
+                    cipheredDataKey: Buffer.from(
+                        objectGetInfo.cipheredDataKey, 'base64'),
+                };
+                const offset = objectGetInfo.range ? objectGetInfo.range[0] : 0;
+                return kms.createDecipherBundle(serverSideEncryption,
+                    offset, log, (err, decipherBundle) => {
+                        if (err) {
+                            log.error('cannot get decipher bundle from kms',
+                                { method: 'objectGet' });
+                            return next(err);
+                        }
+                        // eslint-disable-next-line no-param-reassign
+                        objectGetInfo.decipherStream = decipherBundle.decipher;
+                        return next();
+                    });
+            },
+            err => {
+                if (err) {
+                    return callback(err);
+                }
+
+                return data.head(dataLocator, log, err => {
+                    if (err) {
+                        log.error('error from external backend checking for ' +
+                        'object existence', { error: err });
+                        return callback(err);
+                    }
+                    pushMetric('getObject', log, {
+                        authInfo,
+                        bucket: bucketName,
+                        keys: [objectKey],
+                        newByteLength:
+                            Number.parseInt(responseMetaHeaders['Content-Length'], 10),
+                        versionId: objMD.versionId,
+                        location: objMD.dataStoreName,
+                    });
+                    return callback(null, dataLocator, responseMetaHeaders,
+                        byteRange);
+                });
             }
-            pushMetric('getObject', log, {
-                authInfo,
-                bucket: bucketName,
-                keys: [objectKey],
-                newByteLength:
-                    Number.parseInt(responseMetaHeaders['Content-Length'], 10),
-                versionId: objMD.versionId,
-                location: objMD.dataStoreName,
-            });
-            return callback(null, dataLocator, responseMetaHeaders,
-                byteRange);
-        });
+        );
     });
 }
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "s3",
-  "version": "7.10.60",
+  "version": "7.10.61",
   "description": "S3 connector",
   "main": "index.js",
   "engines": {
@@ -20,7 +20,7 @@
   "homepage": "https://github.com/scality/S3#readme",
   "dependencies": {
     "@hapi/joi": "^17.1.0",
-    "arsenal": "git+https://github.com/scality/arsenal#7.10.67",
+    "arsenal": "git+https://github.com/scality/arsenal#7.10.68",
     "async": "~2.5.0",
     "aws-sdk": "2.905.0",
     "azure-storage": "^2.1.0",

tests/unit/api/objectGet.js

@@ -1,15 +1,19 @@
 const assert = require('assert');
 const async = require('async');
 const crypto = require('crypto');
+const sinon = require('sinon');
+const { errors } = require('arsenal');
 const { parseString } = require('xml2js');
 
 const { bucketPut } = require('../../../lib/api/bucketPut');
 const { cleanup, DummyRequestLogger, makeAuthInfo } = require('../helpers');
 const completeMultipartUpload
     = require('../../../lib/api/completeMultipartUpload');
+const { data } = require('../../../lib/data/wrapper');
 const DummyRequest = require('../DummyRequest');
 const initiateMultipartUpload
     = require('../../../lib/api/initiateMultipartUpload');
+const kms = require('../../../lib/kms/wrapper');
 const objectPut = require('../../../lib/api/objectPut');
 const objectGet = require('../../../lib/api/objectGet');
 const objectPutPart = require('../../../lib/api/objectPutPart');
@@ -390,4 +394,45 @@ describe('objectGet API', () => {
                 });
         });
     });
+
+    describe('with an encrypted object', () => {
+        const testPutEncryptedObjectRequest = () => new DummyRequest({
+            bucketName,
+            namespace,
+            objectKey: objectName,
+            headers: {
+                'x-amz-server-side-encryption': 'AES256',
+                'content-length': '12',
+            },
+            parsedContentLength: 12,
+            url: `/${bucketName}/${objectName}`,
+        }, postBody);
+
+        afterEach(() => {
+            sinon.restore();
+        });
+
+        it('should return KMS error before checking data', done => {
+            const kmsError = errors.InternalError
+                .customizeDescription('KMS key is not usable');
+            async.waterfall([
+                next => bucketPut(authInfo, testPutBucketRequest, log,
+                    err => next(err)),
+                next => objectPut(authInfo, testPutEncryptedObjectRequest(),
+                    undefined, log, err => next(err)),
+            ], err => {
+                assert.ifError(err);
+                const createDecipherBundleStub = sinon
+                    .stub(kms, 'createDecipherBundle')
+                    .callsFake((sse, offset, log, cb) => cb(kmsError));
+                const dataHeadSpy = sinon.spy(data, 'head');
+                objectGet(authInfo, testGetRequest, false, log, err => {
+                    assert.strictEqual(err, kmsError);
+                    assert(createDecipherBundleStub.calledOnce);
+                    assert(dataHeadSpy.notCalled);
+                    done();
+                });
+            });
+        });
+    });
 });

yarn.lock

@@ -488,9 +488,9 @@ arraybuffer.slice@~0.0.7:
   optionalDependencies:
     ioctl "^2.0.2"
 
-"arsenal@git+https://github.com/scality/arsenal#7.10.67":
-  version "7.10.67"
-  resolved "git+https://github.com/scality/arsenal#4d12025b5dd95e67a3bb9e7f9e05058456b561d7"
+"arsenal@git+https://github.com/scality/arsenal#7.10.68":
+  version "7.10.68"
+  resolved "git+https://github.com/scality/arsenal#931e3f1ed0ad5f09d2b770ee19895333a2662445"
   dependencies:
     "@types/async" "^3.2.12"
     "@types/utf8" "^3.0.1"