CLDSRV-1058: Replace internal bucket keys that carry an ARN prefix during SSE migration

lib/api/apiUtils/bucket/updateEncryption.js

@@ -16,10 +16,9 @@ function updateBucketEncryption(bucket, log, cb) {
     const masterKey = sse.masterKeyId;
     const configuredKey = sse.configuredMasterKeyId;
 
-    // Note: if migration is from an external to an external, absence of arn is not enough
-    // a comparison of arn will be necessary but config validation blocks this for now
-    const updateMaster = masterKey && !isScalityKmsArn(masterKey);
-    const updateConfigured = configuredKey && !isScalityKmsArn(configuredKey);
+    // Replace keys of the KMS that SSE migration moves away from
+    const updateMaster = masterKey && kms.isMigrationSourceKey(masterKey);
+    const updateConfigured = configuredKey && kms.isMigrationSourceKey(configuredKey);
 
     if (!updateMaster && !updateConfigured) {
         return cb(null, bucket);
@@ -87,8 +86,7 @@ function updateObjectEncryption(bucket, objMD, objectKey, log, keyArnPrefix, opt
     if (opts.skipObjectUpdate) {
         return cb(null, bucket, objMD);
     }
-    return metadata.putObjectMD(bucket.getName(), objectKey, objMD, params,
-    log, err => cb(err, bucket, objMD));
+    return metadata.putObjectMD(bucket.getName(), objectKey, objMD, params, log, err => cb(err, bucket, objMD));
 }
 
 /**

lib/kms/wrapper.js

@@ -70,6 +70,10 @@ function getClient(kms) {
     return impl();
 }
 
+function getClientIdentifier(backend) {
+    return `${backend.type}:${backend.protocol}:${backend.provider}`;
+}
+
 /**
  * Note: non current instance from previous keys won't be healthchecked
  * `{ [`type:protocol:provider`]: clientDetails }`
@@ -155,6 +159,21 @@ function getClientForKey(key, log) {
     };
 }
 
+/**
+ * Tell whether a stored key belongs to the KMS that SSE migration moves away from.
+ * A key ID without arn prefix belongs to the SSE-migration KMS.
+ * @param {string} storedKey - KeyId or KeyArn from metadata
+ * @param {object} [migrationBackend] - backend of the SSE-migration KMS, if configured
+ * @returns {boolean} true if the key belongs to the SSE-migration KMS
+ */
+function belongsToMigrationSource(storedKey, migrationBackend) {
+    if (!migrationBackend) {
+        return false;
+    }
+    const detail = extractDetailFromArn(storedKey);
+    return !detail.type || getClientIdentifier(detail) === getClientIdentifier(migrationBackend);
+}
+
 class KMS {
     /** Access to client for tests (to create a key without vault check and bucket) */
     static get client() {
@@ -330,6 +349,15 @@ class KMS {
         return !a.error && !b.error && a.clientIdentifier === b.clientIdentifier && a.key === b.key;
     }
 
+    /**
+     * Tell whether a stored key belongs to the KMS that SSE migration moves away from
+     * @param {string} storedKey - KeyId or KeyArn from metadata
+     * @returns {boolean} true if a key of the default KMS must replace it
+     */
+    static isMigrationSourceKey(storedKey) {
+        return belongsToMigrationSource(storedKey, previousBackend);
+    }
+
     /**
      * createCipherBundle
      * @param {object} serverSideEncryptionInfo - info for encryption
@@ -604,3 +632,4 @@ class KMS {
 }
 
 module.exports = KMS;
+module.exports.belongsToMigrationSource = belongsToMigrationSource;

package.json

@@ -1,6 +1,6 @@
 {
     "name": "@zenko/cloudserver",
-    "version": "9.3.29",
+    "version": "9.3.30",
     "description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol",
     "main": "index.js",
     "engines": {
@@ -134,7 +134,7 @@
         "ft_kmip_cluster": "cd tests/functional/sse-kms-migration && mocha --reporter mocha-multi-reporters --reporter-options configFile=$INIT_CWD/tests/reporter-config.json -t 300000 load.js --exit",
         "ft_sse_cleanup": "cd tests/functional/sse-kms-migration && mocha --reporter mocha-multi-reporters --reporter-options configFile=$INIT_CWD/tests/reporter-config.json -t 10000 cleanup.js --exit",
         "ft_sse_before_migration": "cd tests/functional/sse-kms-migration && mocha --reporter mocha-multi-reporters --reporter-options configFile=$INIT_CWD/tests/reporter-config.json -t 10000 cleanup.js beforeMigration.js --exit",
-        "ft_sse_migration": "cd tests/functional/sse-kms-migration && mocha --reporter mocha-multi-reporters --reporter-options configFile=$INIT_CWD/tests/reporter-config.json -t 10000 migration.js --exit",
+        "ft_sse_migration": "cd tests/functional/sse-kms-migration && mocha --reporter mocha-multi-reporters --reporter-options configFile=$INIT_CWD/tests/reporter-config.json -t 10000 migration.js internalBucketKeys.js --exit",
         "ft_sse_arn": "cd tests/functional/sse-kms-migration && mocha --reporter mocha-multi-reporters --reporter-options configFile=$INIT_CWD/tests/reporter-config.json -t 10000 cleanup.js arnPrefix.js --exit",
         "lint": "eslint $(git ls-files '*.js')",
         "lint_md": "mdlint $(git ls-files '*.md')",

tests/functional/sse-kms-migration/internalBucketKeys.js

@@ -0,0 +1,106 @@
+const assert = require('assert');
+const kms = require('../../../lib/kms/wrapper');
+const filekms = require('../../../lib/kms/file/backend');
+const { DummyRequestLogger } = require('../../unit/helpers');
+const helpers = require('./helpers');
+
+const log = new DummyRequestLogger();
+const arnPrefixReg = new RegExp(`^${kms.arnPrefix}`);
+
+function createInternalKeyArn() {
+    return new Promise((resolve, reject) =>
+        filekms.createBucketKey('bucket', log, (err, keyId, keyArn) => (err ? reject(err) : resolve(keyArn))),
+    );
+}
+
+async function getBucketSSEMD(Bucket) {
+    const bucketMD = await helpers.MD.getBucket(Bucket, log);
+    return bucketMD.getServerSideEncryption();
+}
+
+describe('SSE KMS migration of internal bucket keys with ARN prefix', () => {
+    before(async () => helpers.MD.setup());
+
+    [
+        {
+            name: 'created by S3C',
+            Bucket: 'enc-bkt-internal-arn-master-key',
+            keyField: 'masterKeyId',
+            setBucketKey: async (Bucket, keyArn) => {
+                const bucketMD = await helpers.MD.getBucket(Bucket, log);
+                bucketMD.setServerSideEncryption({
+                    cryptoScheme: 1,
+                    algorithm: 'aws:kms',
+                    mandatory: true,
+                    masterKeyId: keyArn,
+                });
+                await helpers.MD.updateBucket(Bucket, bucketMD, log);
+            },
+        },
+        {
+            name: 'set by the user',
+            Bucket: 'enc-bkt-internal-arn-configured-key',
+            keyField: 'configuredMasterKeyId',
+            setBucketKey: (Bucket, keyArn) =>
+                helpers.s3.putBucketEncryption({
+                    Bucket,
+                    ServerSideEncryptionConfiguration: helpers.hydrateSSEConfig({
+                        algo: 'aws:kms',
+                        masterKeyId: keyArn,
+                    }),
+                }),
+        },
+    ].forEach(({ name, Bucket, keyField, setBucketKey }) =>
+        describe(`bucket key ${name}`, () => {
+            const oldObj = { Key: 'old-obj', Body: 'BODY(old-obj)' };
+            const newObj = { Key: 'new-obj', Body: 'BODY(new-obj)' };
+            let internalKeyArn;
+            let replacedSSE;
+
+            before(async () => {
+                internalKeyArn = await createInternalKeyArn();
+                await helpers.s3.createBucket({ Bucket });
+                // Put before the bucket has a key: an object request would replace it
+                await helpers.putEncryptedObject(
+                    Bucket,
+                    oldObj.Key,
+                    { algo: 'aws:kms', masterKeyId: true },
+                    internalKeyArn,
+                    oldObj.Body,
+                );
+                await setBucketKey(Bucket, internalKeyArn);
+                const sse = await getBucketSSEMD(Bucket);
+                assert.strictEqual(sse[keyField], internalKeyArn);
+            });
+
+            after(async () => helpers.cleanup(Bucket));
+
+            it('should replace the bucket key by a key of the default KMS on HeadObject', async () => {
+                await helpers.s3.headObject({ Bucket, Key: oldObj.Key });
+                replacedSSE = await getBucketSSEMD(Bucket);
+                assert.match(replacedSSE[keyField], arnPrefixReg);
+            });
+
+            it('should keep the internal key of an existing object and decrypt it', async () => {
+                const sseMD = await helpers.getObjectMDSSE(Bucket, oldObj.Key);
+                assert.strictEqual(sseMD.SSEKMSKeyId, internalKeyArn);
+                const get = await helpers.s3.getObject({ Bucket, Key: oldObj.Key });
+                assert.strictEqual(get.Body, oldObj.Body);
+            });
+
+            it('should encrypt a new object with the replacing key', async () => {
+                await helpers.s3.putObject({ Bucket, ...newObj });
+                const sseMD = await helpers.getObjectMDSSE(Bucket, newObj.Key);
+                assert.match(sseMD.SSEKMSKeyId, arnPrefixReg);
+                assert.strictEqual(sseMD.SSEKMSKeyId, replacedSSE[keyField]);
+                const get = await helpers.s3.getObject({ Bucket, Key: newObj.Key });
+                assert.strictEqual(get.Body, newObj.Body);
+            });
+
+            it('should keep the replacing key on later requests', async () => {
+                await helpers.s3.headObject({ Bucket, Key: oldObj.Key });
+                assert.deepStrictEqual(await getBucketSSEMD(Bucket), replacedSSE);
+            });
+        }),
+    );
+});

tests/unit/encryption/belongsToMigrationSource.js

@@ -0,0 +1,47 @@
+const assert = require('assert');
+const { KmsType, KmsProtocol, makeBackend } = require('arsenal/build/lib/network/KMSInterface');
+const { belongsToMigrationSource } = require('../../../lib/kms/wrapper');
+
+const internal = makeBackend(KmsType.internal, KmsProtocol.file, 'scality');
+const kmip = makeBackend(KmsType.external, KmsProtocol.kmip, 'thales');
+const awsKms = makeBackend(KmsType.external, KmsProtocol.aws_kms, 'localaws');
+const keyId = '891a72205529b8a7531362d59d485f2fd182bb17cc7b07b2613fb01a2f966d56';
+
+describe('belongsToMigrationSource', () => {
+    [
+        {
+            desc: 'a key ID without ARN prefix',
+            storedKey: keyId,
+            migrationBackend: internal,
+            expected: true,
+        },
+        {
+            desc: 'a key ARN of the SSE-migration KMS',
+            storedKey: `${internal.arnPrefix}${keyId}`,
+            migrationBackend: internal,
+            expected: true,
+        },
+        {
+            desc: 'a key ARN of a KMIP KMS',
+            storedKey: `${kmip.arnPrefix}931`,
+            migrationBackend: internal,
+            expected: false,
+        },
+        {
+            desc: 'a key ARN of an AWS KMS',
+            storedKey: `${awsKms.arnPrefix}36d31b52-c5a7-4e1d-9f0b-2a8c3e6d7f41`,
+            migrationBackend: internal,
+            expected: false,
+        },
+        {
+            desc: 'a key ARN of the internal KMS without SSE migration',
+            storedKey: `${internal.arnPrefix}${keyId}`,
+            migrationBackend: undefined,
+            expected: false,
+        },
+    ].forEach(({ desc, storedKey, migrationBackend, expected }) =>
+        it(`should return ${expected} for ${desc}`, () => {
+            assert.strictEqual(belongsToMigrationSource(storedKey, migrationBackend), expected);
+        }),
+    );
+});