From b004c1bc08e7ba7e524bb922829b6ef9127217dd Mon Sep 17 00:00:00 2001 From: Mickael Bourgois Date: Thu, 3 Apr 2025 17:28:58 +0200 Subject: [PATCH 1/4] CLDSRV-629: Handle KMS before data to return error (cherry picked from commit 5184ec2ad4cc5951c6c5d421ae7c786f1f9579e7) Dropped monitoring, as it appeared only in 7.70 and not 7.10 This goes with https://github.com/scality/Arsenal/pull/2720 For RD-2378 --- lib/api/objectGet.js | 69 +++++++++++++++++++++++++++++++++----------- 1 file changed, 52 insertions(+), 17 deletions(-) diff --git a/lib/api/objectGet.js b/lib/api/objectGet.js index 2e736d199a..3f3b84c6da 100644 --- a/lib/api/objectGet.js +++ b/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); - }); + ); }); } From dc96eddf6b92fe2ecfc23d527eab0b05302e8089 Mon Sep 17 00:00:00 2001 From: Mickael Bourgois Date: Fri, 2 Oct 2026 15:47:42 +0200 Subject: [PATCH 2/4] CLDSRV-1012: Bump arsenal for backports --- package.json | 2 +- yarn.lock | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 7ee15276bc..f25716e37a 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/yarn.lock b/yarn.lock index ccc806800e..f11b18c407 100644 --- a/yarn.lock +++ b/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" From 50353035d69cf666eb9d7fb98fcc40983540b6fb Mon Sep 17 00:00:00 2001 From: Mickael Bourgois Date: Wed, 7 Oct 2026 23:56:04 +0200 Subject: [PATCH 3/4] CLDSRV-1012: Test return KMS error on GetObject --- tests/unit/api/objectGet.js | 45 +++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/unit/api/objectGet.js b/tests/unit/api/objectGet.js index f869928635..19f47bf62e 100644 --- a/tests/unit/api/objectGet.js +++ b/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(); + }); + }); + }); + }); }); From a5b4c80e3c0b7033c81d28d11ef47346f78f1b69 Mon Sep 17 00:00:00 2001 From: Mickael Bourgois Date: Wed, 7 Oct 2026 23:56:04 +0200 Subject: [PATCH 4/4] CLDSRV-1012: Bump version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index f25716e37a..9e6808f4bf 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "s3", - "version": "7.10.60", + "version": "7.10.61", "description": "S3 connector", "main": "index.js", "engines": {