CLDSRV-1021: Compare s3:VersionId with the copy source version ID as sent

- Use the copy source version ID as sent by the client, for both the bucket
  policy and the authorization request, instead of the internal version ID
- Return InvalidArgument when the copy source versionId is repeated, so an
  array never reaches the bucket policy conditions

Author: tcarmet

lib/api/api.js

@@ -258,7 +258,7 @@ function callApiHandler(apiMethod, apiHandler, request, response, log, callback)
         return apiHandler(request, log, callback);
     }
 
-    const { sourceBucket, sourceObject, sourceVersionId, parsingError } = parseCopySource(
+    const { sourceBucket, sourceObject, sourceVersionId, copySourceVersionId, parsingError } = parseCopySource(
         apiMethod,
         request.headers['x-amz-copy-source'],
     );
@@ -277,7 +277,7 @@ function callApiHandler(apiMethod, apiHandler, request, response, log, callback)
         return process.nextTick(callback, httpHeadersSizeError);
     }
 
-    const requestContexts = prepareRequestContexts(apiMethod, request, sourceBucket, sourceObject, sourceVersionId);
+    const requestContexts = prepareRequestContexts(apiMethod, request, sourceBucket, sourceObject, copySourceVersionId);
 
     // Extract all the _apiMethods and store them in an array
     const apiMethods = requestContexts ? requestContexts.map(context => context._apiMethod) : [];
@@ -441,6 +441,9 @@ function callApiHandler(apiMethod, apiHandler, request, response, log, callback)
                 return apiHandler(userInfo, request, streamingV4Params, log, methodCallback, authorizationResults);
             }
             if (apiMethod === 'objectCopy' || apiMethod === 'objectPutCopyPart') {
+                // The bucket policy is evaluated against the version ID as
+                // sent, the decoded one is for the metadata lookup
+                request.copySourceVersionId = copySourceVersionId;
                 return apiHandler(userInfo, request, sourceBucket, sourceObject, sourceVersionId, log, methodCallback);
             }
             if (apiMethod === 'objectGet') {

lib/api/apiUtils/authorization/permissionChecks.js

@@ -486,16 +486,18 @@ function checkBucketPolicy(
     // differs from the one the request targets (e.g. the source of a copy)
     let bucketName = request?.bucketName;
     let objectKey = request?.objectKey;
+    let query = request?.query;
     if (policyTarget) {
         bucketName = policyTarget.bucketName;
         objectKey = policyTarget.objectKey;
+        query = Object.assign({}, query, { versionId: policyTarget.versionId });
     }
     const ip = request ? requestUtils.getClientIp(request, config) : undefined;
     const isSecure = request ? requestUtils.getHttpProtocolSecurity(request, config) : undefined;
     const requestContext = request
         ? new RequestContext(
               request.headers,
-              request.query,
+              query,
               bucketName,
               objectKey,
               ip,

lib/api/apiUtils/authorization/prepareRequestContexts.js

@@ -33,7 +33,8 @@ function isHeaderAcl(headers) {
  * @param {object} request - request object
  * @param {string} sourceBucket - name of sourceBucket if copy request
  * @param {string} sourceObject - name of sourceObject if copy request
- * @param {string} sourceVersionId - value of sourceVersionId if copy request
+ * @param {string} sourceVersionId - version ID of the copy source as sent by
+ * the client, if copy request
  * @return {RequestContext []} array of requestContexts
  */
 function prepareRequestContexts(apiMethod, request, sourceBucket, sourceObject, sourceVersionId) {

lib/api/apiUtils/object/parseCopySource.js

@@ -7,7 +7,8 @@ const { decodeVersionId } = require('./versioning');
 /** parseCopySource - parse objectCopy or objectPutCopyPart copy source header
  * @param {string} apiMethod - api method
  * @param {string} copySourceHeader - 'x-amz-copy-source' request header
- * @return {object} - sourceBucket, sourceObject, sourceVersionId, parsingError
+ * @return {object} - sourceBucket, sourceObject, sourceVersionId (decoded),
+ * copySourceVersionId (as sent by the client), parsingError
  */
 function parseCopySource(apiMethod, copySourceHeader) {
     if (apiMethod !== 'objectCopy' && apiMethod !== 'objectPutCopyPart') {
@@ -26,14 +27,21 @@ function parseCopySource(apiMethod, copySourceHeader) {
     // Pull the source bucket and source object separated by /
     const sourceBucket = source.slice(0, slashSeparator);
     const sourceObject = source.slice(slashSeparator + 1);
-    const sourceVersionId =
-        decodeVersionId(query ? querystring.parse(query) : undefined);
+    const sourceQuery = query ? querystring.parse(query) : undefined;
+    // A repeated versionId is parsed as an array
+    if (sourceQuery?.versionId !== undefined && typeof sourceQuery.versionId !== 'string') {
+        return { parsingError: errors.InvalidArgument };
+    }
+    const sourceVersionId = decodeVersionId(sourceQuery);
     if (sourceVersionId instanceof Error) {
         const err = sourceVersionId;
         return { parsingError: err };
     }
+    // Authorization compares s3:VersionId with the version ID as the client
+    // sent it, like for a direct GetObject
+    const copySourceVersionId = sourceQuery?.versionId;
 
-    return { sourceBucket, sourceObject, sourceVersionId };
+    return { sourceBucket, sourceObject, sourceVersionId, copySourceVersionId };
 }
 
 module.exports = parseCopySource;

lib/api/objectCopy.js

@@ -244,8 +244,8 @@ function objectCopy(authInfo, request, sourceBucket, sourceObject, sourceVersion
         checkQuota: true,
         request,
         // The request targets the destination, the bucket policy must be
-        // evaluated against the source
-        policyTarget: { bucketName: sourceBucket, objectKey: sourceObject },
+        // evaluated against the source, with its version ID as sent
+        policyTarget: { bucketName: sourceBucket, objectKey: sourceObject, versionId: request.copySourceVersionId },
     };
     const valPutParams = {
         authInfo,

lib/api/objectPutCopyPart.js

@@ -57,8 +57,8 @@ function objectPutCopyPart(authInfo, request, sourceBucket, sourceObject, reqVer
         checkQuota: true,
         request,
         // The request targets the destination, the bucket policy must be
-        // evaluated against the source
-        policyTarget: { bucketName: sourceBucket, objectKey: sourceObject },
+        // evaluated against the source, with its version ID as sent
+        policyTarget: { bucketName: sourceBucket, objectKey: sourceObject, versionId: request.copySourceVersionId },
     };
 
     const partNumber = Number.parseInt(request.query.partNumber, 10);

lib/metadata/metadataUtils.js

@@ -357,6 +357,7 @@ function checkRateLimitIfNeeded(request, authInfo, bucketMD, log, callback) {
  *   when it differs from the request target, e.g. the source of a copy
  * @param {string} params.policyTarget.bucketName - name of bucket
  * @param {string} params.policyTarget.objectKey - name of object
+ * @param {string} [params.policyTarget.versionId] - version ID as sent by the client
  * @param {boolean} actionImplicitDenies - identity authorization results
  * @param {RequestLogger} log - request logger
  * @param {function} callback - callback