From e4c55ee5ab17ca931361c4a7c8ed54608351653e Mon Sep 17 00:00:00 2001 From: Nicolas Humbert Date: Fri, 15 Nov 2024 16:41:18 +0100 Subject: [PATCH] CLDSRV-580 Custom KMS key id persisted after deleting bucket encryption config (cherry picked from commit 06b252860885df01fcd1c29cb7eefd6c04aab082) --- lib/api/bucketDeleteEncryption.js | 31 +++++++++++++++--------- tests/unit/api/bucketDeleteEncryption.js | 18 ++++++-------- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/lib/api/bucketDeleteEncryption.js b/lib/api/bucketDeleteEncryption.js index 242bea09a4..35bf57c6c9 100644 --- a/lib/api/bucketDeleteEncryption.js +++ b/lib/api/bucketDeleteEncryption.js @@ -30,24 +30,33 @@ function bucketDeleteEncryption(authInfo, request, log, callback) { (bucket, next) => checkExpectedBucketOwner(request.headers, bucket, log, err => next(err, bucket)), (bucket, next) => { const sseConfig = bucket.getServerSideEncryption(); + if (sseConfig === null) { return next(null, bucket); } - const updatedConfig = { - mandatory: false, - algorithm: sseConfig.algorithm, - cryptoScheme: sseConfig.cryptoScheme, - masterKeyId: sseConfig.masterKeyId, - configuredMasterKeyId: sseConfig.configuredMasterKeyId, - }; + const { isAccountEncryptionEnabled, masterKeyId, algorithm, cryptoScheme } = sseConfig; + + let updatedSseConfig = null; - const { isAccountEncryptionEnabled } = sseConfig; - if (isAccountEncryptionEnabled) { - updatedConfig.isAccountEncryptionEnabled = isAccountEncryptionEnabled; + if (!isAccountEncryptionEnabled && masterKeyId) { + // Keep the encryption configuration as a "cache" to avoid generating a new master key: + // - if the default encryption master key is defined at the bucket level (!isAccountEncryptionEnabled), + // - and if a bucket-level default encryption key is already set. + // This "cache" is implemented by storing the configuration in the bucket metadata + // with mandatory set to false, making sure it remains hidden for `getBucketEncryption` operations. + // There is no need to cache the configuration if the default encryption master key is + // managed at the account level, as the master key id in that case is stored directly in + // the account metadata. + updatedSseConfig = { + mandatory: false, + algorithm, + cryptoScheme, + masterKeyId, + }; } - bucket.setServerSideEncryption(updatedConfig); + bucket.setServerSideEncryption(updatedSseConfig); return metadata.updateBucket(bucketName, bucket, log, err => next(err, bucket)); }, ], diff --git a/tests/unit/api/bucketDeleteEncryption.js b/tests/unit/api/bucketDeleteEncryption.js index f9453d7162..5d94b49f55 100644 --- a/tests/unit/api/bucketDeleteEncryption.js +++ b/tests/unit/api/bucketDeleteEncryption.js @@ -51,7 +51,7 @@ describe('bucketDeleteEncryption API', () => { }); })); - it('should disable mandatory sse and clear key for aws:kms with a configured master key id', done => { + it('should remove sse and clear key for aws:kms with a configured master key id', done => { const post = templateSSEConfig({ algorithm: 'aws:kms', keyId: '12345' }); bucketPutEncryption(authInfo, templateRequest(bucketName, { post }), log, err => { assert.ifError(err); @@ -59,9 +59,7 @@ describe('bucketDeleteEncryption API', () => { assert.ifError(err); return getSSEConfig(bucketName, log, (err, sseInfo) => { assert.ifError(err); - assert(!sseInfo.masterKeyId); - assert.strictEqual(sseInfo.mandatory, false); - assert.strictEqual(sseInfo.configuredMasterKeyId, '12345'); + assert(!sseInfo); done(); }); }); @@ -228,7 +226,7 @@ describe('bucketDeleteEncryption API', () => { sinon.restore(); }); - it('should keep isAccountEncryptionEnabled after deleting AES256 bucket encryption', done => { + it('should clear the sse config after deleting AES256 bucket encryption', done => { const post = templateSSEConfig({ algorithm: 'AES256' }); bucketPutEncryption(authInfo, templateRequest(bucketName, { post }), log, err => { assert.ifError(err); @@ -239,7 +237,7 @@ describe('bucketDeleteEncryption API', () => { assert.ifError(err); return getSSEConfig(bucketName, log, (err, sseInfoAfterDeletion) => { assert.ifError(err); - assert.strictEqual(sseInfoAfterDeletion.isAccountEncryptionEnabled, true); + assert(!sseInfoAfterDeletion); done(); }); }); @@ -247,7 +245,7 @@ describe('bucketDeleteEncryption API', () => { }); }); - it('should keep isAccountEncryptionEnabled after deleting aws:kms bucket encryption', done => { + it('should clear the sse config after deleting aws:kms bucket encryption', done => { const post = templateSSEConfig({ algorithm: 'aws:kms' }); bucketPutEncryption(authInfo, templateRequest(bucketName, { post }), log, err => { assert.ifError(err); @@ -258,7 +256,7 @@ describe('bucketDeleteEncryption API', () => { assert.ifError(err); return getSSEConfig(bucketName, log, (err, sseInfoAfterDeletion) => { assert.ifError(err); - assert.strictEqual(sseInfoAfterDeletion.isAccountEncryptionEnabled, true); + assert(!sseInfoAfterDeletion); done(); }); }); @@ -266,7 +264,7 @@ describe('bucketDeleteEncryption API', () => { }); }); - it('should keep isAccountEncryptionEnabled after deleting aws:kms and key id bucket encryption', done => { + it('should clear the sse config after deleting aws:kms and key id bucket encryption', done => { const postAES256 = templateSSEConfig({ algorithm: 'AES256' }); bucketPutEncryption(authInfo, templateRequest(bucketName, { post: postAES256 }), log, err => { assert.ifError(err); @@ -280,7 +278,7 @@ describe('bucketDeleteEncryption API', () => { assert.ifError(err); return getSSEConfig(bucketName, log, (err, sseInfoAfterDeletion) => { assert.ifError(err); - assert.strictEqual(sseInfoAfterDeletion.isAccountEncryptionEnabled, true); + assert(!sseInfoAfterDeletion); done(); }); });