diff --git a/lib/api/apiUtils/bucket/updateEncryption.js b/lib/api/apiUtils/bucket/updateEncryption.js index 5db3152423..b8a01ced92 100644 --- a/lib/api/apiUtils/bucket/updateEncryption.js +++ b/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)); } /** diff --git a/lib/kms/wrapper.js b/lib/kms/wrapper.js index 4236bd8563..125623f0ee 100644 --- a/lib/kms/wrapper.js +++ b/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; diff --git a/package.json b/package.json index 9045704cb4..682b347583 100644 --- a/package.json +++ b/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')", diff --git a/tests/functional/sse-kms-migration/internalBucketKeys.js b/tests/functional/sse-kms-migration/internalBucketKeys.js new file mode 100644 index 0000000000..554f682738 --- /dev/null +++ b/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); + }); + }), + ); +}); diff --git a/tests/unit/encryption/belongsToMigrationSource.js b/tests/unit/encryption/belongsToMigrationSource.js new file mode 100644 index 0000000000..4052f86b0f --- /dev/null +++ b/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); + }), + ); +});