From b3f446aeb97659667670518cc2de95469a52c608 Mon Sep 17 00:00:00 2001 From: Nicolas Humbert Date: Fri, 22 Nov 2024 16:53:30 +0100 Subject: [PATCH] CLDSRV-584 Limit backbeat API versioning check to replication operations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: In Artesca, when creating a location (whether used for replication or not), you specify both the endpoint and the bucket name where data will be stored. At this stage, Zenko checks that the bucket has versioning enabled. In S3C, it is a bit different, a “replication location” consists of a list of S3C endpoints (replicationEndpoints), and the destination bucket is specified later in the API replication rule (with put-bucket-replication API). Currently, S3C does not check if the destination bucket has versioning enabled. This means data could be replicated from a versioned source bucket to a non-versioned or suspended destination bucket, which can cause issues. e.g. https://scality.atlassian.net/browse/S3C-821 Current solution: A validation step was added to the Backbeat API in cloudserver. This check rejects any requests made to buckets that either do not have versioning enabled or have versioning suspended. This check makes sure the Backbeat API only works with buckets that have versioning enabled. This prevents operations on destination buckets where versioning is disabled or suspended, hense maintaining proper version control. On the source, we should be fine. Cloudserver prevents setting up replication on a bucket if its versioning is disabled or suspended, and it prevents changing a bucket’s versioning to suspended while replication is ongoing. Issue: The Backbeat API is also used for lifecycle, which works on both versioned and non-versioned buckets. The current versioning check affects the lifecycle operations, which should be allowed on non-versioned buckets. Proposed solution: Modify the Backbeat API to apply the bucket versioning check only to actions related to the replication destination buckets (such as PUT, POST, and DELETE requests). Implement a new client header x-scal-versioning-enabled in the Backbeat client to make sure that replication targets only versioned buckets. When this header is set, Cloudserver will make sure that the destination bucket has versioning enabled, preventing replication to non-versioned buckets. This change will ensure that replication only targets versioned buckets and allows lifecycle operations on non-versioned buckets. --- lib/routes/routeBackbeat.js | 5 +- .../raw-node/test/routes/routeBackbeat.js | 8 +- tests/unit/routes/routeBackbeat.js | 185 ++++++++++++++++++ 3 files changed, 195 insertions(+), 3 deletions(-) create mode 100644 tests/unit/routes/routeBackbeat.js diff --git a/lib/routes/routeBackbeat.js b/lib/routes/routeBackbeat.js index c2225b6a76..d208152655 100644 --- a/lib/routes/routeBackbeat.js +++ b/lib/routes/routeBackbeat.js @@ -1289,7 +1289,10 @@ function routeBackbeat(clientIP, request, response, log) { [request.query.operation](request, response, log, next); } const versioningConfig = bucketInfo.getVersioningConfiguration(); - if (!versioningConfig || versioningConfig.Status !== 'Enabled') { + // The following makes sure that only replication destination-related operations + // target buckets with versioning enabled. + const isVersioningEnabled = request.headers['x-scal-versioning-enabled'] === 'true'; + if (isVersioningEnabled && (!versioningConfig || versioningConfig.Status !== 'Enabled')) { log.debug('bucket versioning is not enabled', { method: request.method, bucketName: request.bucketName, diff --git a/tests/functional/raw-node/test/routes/routeBackbeat.js b/tests/functional/raw-node/test/routes/routeBackbeat.js index fecd8b95e3..5e14921b89 100644 --- a/tests/functional/raw-node/test/routes/routeBackbeat.js +++ b/tests/functional/raw-node/test/routes/routeBackbeat.js @@ -1496,7 +1496,7 @@ describeSkipIfAWS('backbeat routes', () => { }); }); - it('should refuse PUT data if bucket is not versioned', + it('should refuse PUT data if bucket is not versioned and x-scal-versioning-enabled is true', done => makeBackbeatRequest({ method: 'PUT', bucket: NONVERSIONED_BUCKET, objectKey: testKey, resourceType: 'data', @@ -1504,6 +1504,7 @@ describeSkipIfAWS('backbeat routes', () => { headers: { 'content-length': testData.length, 'x-scal-canonical-id': testArn, + 'x-scal-versioning-enabled': 'true', }, authCredentials: backbeatAuthCredentials, requestBody: testData, @@ -1513,13 +1514,16 @@ describeSkipIfAWS('backbeat routes', () => { done(); })); - it('should refuse PUT metadata if bucket is not versioned', + it('should refuse PUT metadata if bucket is not versioned and x-scal-versioning-enabled is true', done => makeBackbeatRequest({ method: 'PUT', bucket: NONVERSIONED_BUCKET, objectKey: testKey, resourceType: 'metadata', queryObj: { versionId: versionIdUtils.encode(testMd.versionId), }, + headers: { + 'x-scal-versioning-enabled': 'true', + }, authCredentials: backbeatAuthCredentials, requestBody: JSON.stringify(testMd), }, diff --git a/tests/unit/routes/routeBackbeat.js b/tests/unit/routes/routeBackbeat.js new file mode 100644 index 0000000000..2ed2d53d68 --- /dev/null +++ b/tests/unit/routes/routeBackbeat.js @@ -0,0 +1,185 @@ +const assert = require('assert'); +const sinon = require('sinon'); +const metadataUtils = require('../../../lib/metadata/metadataUtils'); +const storeObject = require('../../../lib/api/apiUtils/object/storeObject'); +const metadata = require('../../../lib/metadata/wrapper'); +const { DummyRequestLogger } = require('../helpers'); +const DummyRequest = require('../DummyRequest'); + +const log = new DummyRequestLogger(); + +function prepareDummyRequest(headers = {}) { + const request = new DummyRequest({ + hostname: 'localhost', + method: 'PUT', + url: '/_/backbeat/metadata/bucket0/key0', + port: 80, + headers, + socket: { + remoteAddress: '0.0.0.0', + }, + }, '{"replicationInfo":"{}"}'); + return request; +} + +describe('routeBackbeat', () => { + let mockResponse; + let mockRequest; + let sandbox; + let endPromise; + let resolveEnd; + let routeBackbeat; + + beforeEach(() => { + sandbox = sinon.createSandbox(); + + // create a Promise that resolves when response.end is called + endPromise = new Promise((resolve) => { resolveEnd = resolve; }); + + mockResponse = { + statusCode: null, + body: null, + setHeader: () => {}, + writeHead: sandbox.spy(statusCode => { + mockResponse.statusCode = statusCode; + }), + end: sandbox.spy((body, encoding, callback) => { + mockResponse.body = JSON.parse(body); + if (callback) callback(); + resolveEnd(); // Resolve the Promise when end is called + }), + }; + + mockRequest = prepareDummyRequest(); + + sandbox.stub(metadataUtils, 'standardMetadataValidateBucketAndObj'); + sandbox.stub(storeObject, 'dataStore'); + + // Clear require cache for routeBackbeat to make sure fresh module with stubbed dependencies + delete require.cache[require.resolve('../../../lib/routes/routeBackbeat')]; + routeBackbeat = require('../../../lib/routes/routeBackbeat'); + }); + + afterEach(() => { + sandbox.restore(); + }); + + const rejectionTests = [ + { + description: 'should reject CRR destination (putData) requests when versioning is disabled', + method: 'PUT', + url: '/_/backbeat/data/bucket0/key0', + }, + { + description: 'should reject CRR destination (putMetadata) requests when versioning is disabled', + method: 'PUT', + url: '/_/backbeat/metadata/bucket0/key0', + }, + ]; + + rejectionTests.forEach(({ description, method, url }) => { + it(description, async () => { + mockRequest.method = method; + mockRequest.url = url; + mockRequest.headers = { + 'x-scal-versioning-enabled': 'true', + }; + metadataUtils.standardMetadataValidateBucketAndObj.callsFake((params, denies, log, callback) => { + const bucketInfo = { + getVersioningConfiguration: () => ({ Status: 'Disabled' }), + }; + const objMd = {}; + callback(null, bucketInfo, objMd); + }); + + routeBackbeat('127.0.0.1', mockRequest, mockResponse, log); + + void await endPromise; + + assert.strictEqual(mockResponse.statusCode, 409); + assert.strictEqual(mockResponse.body.code, 'InvalidBucketState'); + }); + }); + + it('should allow non-CRR destination (getMetadata) requests regardless of versioning', async () => { + mockRequest.method = 'GET'; + + metadataUtils.standardMetadataValidateBucketAndObj.callsFake((params, denies, log, callback) => { + const bucketInfo = { + getVersioningConfiguration: () => ({ Status: 'Disabled' }), + }; + const objMd = {}; + callback(null, bucketInfo, objMd); + }); + + routeBackbeat('127.0.0.1', mockRequest, mockResponse, log); + + void await endPromise; + + assert.strictEqual(mockResponse.statusCode, 200); + assert.deepStrictEqual(mockResponse.body, { Body: '{}' }); + }); + + it('should allow CRR destination requests (putMetadata) when versioning is enabled', async () => { + mockRequest.method = 'PUT'; + mockRequest.url = '/_/backbeat/metadata/bucket0/key0'; + mockRequest.headers = { + 'x-scal-versioning-enabled': 'true', + }; + mockRequest.destroy = () => {}; + + sandbox.stub(metadata, 'putObjectMD').callsFake((bucketName, objectKey, omVal, options, logParam, cb) => { + cb(null, {}); + }); + + metadataUtils.standardMetadataValidateBucketAndObj.callsFake((params, denies, log, callback) => { + const bucketInfo = { + getVersioningConfiguration: () => ({ Status: 'Enabled' }), + isVersioningEnabled: () => true, + }; + const objMd = {}; + callback(null, bucketInfo, objMd); + }); + + routeBackbeat('127.0.0.1', mockRequest, mockResponse, log); + + void await endPromise; + + assert.strictEqual(mockResponse.statusCode, 200); + assert.deepStrictEqual(mockResponse.body, {}); + }); + + it('should allow CRR destination requests (putData) when versioning is enabled', async () => { + const md5 = '1234'; + mockRequest.method = 'PUT'; + mockRequest.url = '/_/backbeat/data/bucket0/key0'; + mockRequest.headers = { + 'x-scal-canonical-id': 'id', + 'content-md5': md5, + 'content-length': '0', + 'x-scal-versioning-enabled': 'true', + }; + mockRequest.destroy = () => {}; + + metadataUtils.standardMetadataValidateBucketAndObj.callsFake((params, denies, log, callback) => { + const bucketInfo = { + getVersioningConfiguration: () => ({ Status: 'Enabled' }), + isVersioningEnabled: () => true, + getLocationConstraint: () => undefined, + }; + const objMd = {}; + callback(null, bucketInfo, objMd); + }); + storeObject.dataStore.callsFake((objectContext, cipherBundle, stream, size, + streamingV4Params, backendInfo, log, callback) => { + callback(null, {}, md5); + }); + + routeBackbeat('127.0.0.1', mockRequest, mockResponse, log); + + void await endPromise; + + assert.strictEqual(mockResponse.statusCode, 200); + assert.deepStrictEqual(mockResponse.body, [{}]); + }); +});