Skip to content

Commit

Permalink
Ensure backbeat routes are authorizing requests
Browse files Browse the repository at this point in the history 
Today, authorization results are not used. It is because before
having implicit denies, we would have an AccessDenied directly.
To be consistent, we must pass the authorization results.
We thus allow the bucket policies and ACLs to be processed.

Unit tests are added to cover all existing routes.
Routes themselves are not direectly tested, only the router
logic. We must first add tests for backbeat routes before
merging more logic with the normal router.

Issue: CLDSRV-591
  • Loading branch information
@williamlardier
williamlardier committed Dec 10, 2024
1 parent 7c88864 commit c0bb428
Show file tree
Hide file tree
Showing 5 changed files with 457 additions and 103 deletions.
85 changes: 41 additions & 44 deletions lib/api/api.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,45 @@ const monitoringMap = policies.actionMaps.actionMonitoringMapS3;

auth.setHandler(vault);

function checkAuthResults(authResults, apiMethod, log) {
let returnTagCount = true;
const isImplicitDeny = {};
let isOnlyImplicitDeny = true;
if (apiMethod === 'objectGet') {
if (!authResults[0].isAllowed && !authResults[0].isImplicit) {
log.trace('get object authorization denial from Vault');
return errors.AccessDenied;
}
isImplicitDeny[authResults[0].action] = authResults[0].isImplicit;
if (!authResults[1].isAllowed) {
log.trace('get tagging authorization denial ' +
'from Vault');
returnTagCount = false;
}
} else {
for (let i = 0; i < authResults.length; i++) {
isImplicitDeny[authResults[i].action] = true;
if (!authResults[i].isAllowed && !authResults[i].isImplicit) {
// Any explicit deny rejects the current API call
log.trace('authorization denial from Vault');
return errors.AccessDenied;
}
if (authResults[i].isAllowed) {
// If the action is allowed, the result is not implicit
// Deny.
isImplicitDeny[authResults[i].action] = false;
isOnlyImplicitDeny = false;
}
}
}
// These two APIs cannot use ACLs or Bucket Policies, hence, any
// implicit deny from vault must be treated as an explicit deny.
if ((apiMethod === 'bucketPut' || apiMethod === 'serviceGet') && isOnlyImplicitDeny) {
return errors.AccessDenied;
}
return { returnTagCount, isImplicitDeny };
}

/* eslint-disable no-param-reassign */
const api = {
callApiMethod(apiMethod, request, response, log, callback) {
Expand Down Expand Up @@ -152,49 +191,6 @@ const api = {
// eslint-disable-next-line no-param-reassign
request.apiMethods = apiMethods;

function checkAuthResults(authResults) {
let returnTagCount = true;
const isImplicitDeny = {};
let isOnlyImplicitDeny = true;
if (apiMethod === 'objectGet') {
// first item checks s3:GetObject(Version) action
if (!authResults[0].isAllowed && !authResults[0].isImplicit) {
log.trace('get object authorization denial from Vault');
return errors.AccessDenied;
}
// TODO add support for returnTagCount in the bucket policy
// checks
isImplicitDeny[authResults[0].action] = authResults[0].isImplicit;
// second item checks s3:GetObject(Version)Tagging action
if (!authResults[1].isAllowed) {
log.trace('get tagging authorization denial ' +
'from Vault');
returnTagCount = false;
}
} else {
for (let i = 0; i < authResults.length; i++) {
isImplicitDeny[authResults[i].action] = true;
if (!authResults[i].isAllowed && !authResults[i].isImplicit) {
// Any explicit deny rejects the current API call
log.trace('authorization denial from Vault');
return errors.AccessDenied;
}
if (authResults[i].isAllowed) {
// If the action is allowed, the result is not implicit
// Deny.
isImplicitDeny[authResults[i].action] = false;
isOnlyImplicitDeny = false;
}
}
}
// These two APIs cannot use ACLs or Bucket Policies, hence, any
// implicit deny from vault must be treated as an explicit deny.
if ((apiMethod === 'bucketPut' || apiMethod === 'serviceGet') && isOnlyImplicitDeny) {
return errors.AccessDenied;
}
return { returnTagCount, isImplicitDeny };
}

return async.waterfall([
next => auth.server.doAuth(
request, log, (err, userInfo, authorizationResults, streamingV4Params, infos) => {
Expand Down Expand Up @@ -273,7 +269,7 @@ const api = {
}
request.accountQuotas = infos?.accountQuota;
if (authorizationResults) {
const checkedResults = checkAuthResults(authorizationResults);
const checkedResults = checkAuthResults(authorizationResults, apiMethod, log);
if (checkedResults instanceof Error) {
return callback(checkedResults);
}
Expand Down Expand Up @@ -372,6 +368,7 @@ const api = {
serviceGet,
websiteGet: website,
websiteHead: website,
checkAuthResults,
};

module.exports = api;
139 changes: 81 additions & 58 deletions lib/routes/routeBackbeat.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const { listLifecycleNonCurrents } = require('../api/backbeat/listLifecycleNonCu
const { listLifecycleOrphanDeleteMarkers } = require('../api/backbeat/listLifecycleOrphanDeleteMarkers');
const { objectDeleteInternal } = require('../api/objectDelete');
const { validateQuotas } = require('../api/apiUtils/quotas/quotaUtils');
const { checkAuthResults } = require('../api/api');

const { CURRENT_TYPE, NON_CURRENT_TYPE, ORPHAN_DM_TYPE } = constants.lifecycleListing;

Expand Down Expand Up @@ -315,36 +316,6 @@ function handleTaggingOperation(request, response, type, dataStoreVersionId,
});
}

/*
PUT /_/backbeat/metadata/<bucket name>/<object key>
GET /_/backbeat/metadata/<bucket name>/<object key>?versionId=<version id>
PUT /_/backbeat/data/<bucket name>/<object key>
PUT /_/backbeat/multiplebackenddata/<bucket name>/<object key>
?operation=putobject
PUT /_/backbeat/multiplebackenddata/<bucket name>/<object key>
?operation=putpart
DELETE /_/backbeat/multiplebackenddata/<bucket name>/<object key>
?operation=deleteobject
DELETE /_/backbeat/multiplebackenddata/<bucket name>/<object key>
?operation=abortmpu
DELETE /_/backbeat/multiplebackenddata/<bucket name>/<object key>
?operation=deleteobjecttagging
POST /_/backbeat/multiplebackenddata/<bucket name>/<object key>
?operation=initiatempu
POST /_/backbeat/multiplebackenddata/<bucket name>/<object key>
?operation=completempu
POST /_/backbeat/multiplebackenddata/<bucket name>/<object key>
?operation=puttagging
GET /_/backbeat/multiplebackendmetadata/<bucket name>/<object key>
POST /_/backbeat/batchdelete
GET /_/backbeat/lifecycle/<bucket name>?list-type=current
GET /_/backbeat/lifecycle/<bucket name>?list-type=noncurrent
GET /_/backbeat/lifecycle/<bucket name>?list-type=orphan
POST /_/backbeat/index/<bucket name>?operation=add
POST /_/backbeat/index/<bucket name>?operation=delete
DELETE /_/backbeat/index/<bucket name>
*/

function _getLastModified(locations, log, cb) {
const reqUids = log.getSerializedUids();
return dataClient.head(locations, reqUids, (err, data) => {
Expand Down Expand Up @@ -1336,32 +1307,35 @@ const indexEntrySchema = joi.object({

const indexingSchema = joi.array().items(indexEntrySchema).min(1);

function routeIndexingAPIs(request, response, userInfo, log) {
function routeIndexingAPIs(request, response, userInfo, log, callback) {
const route = backbeatRoutes[request.method][request.resourceType];

if (!['GET', 'POST'].includes(request.method)) {
return responseJSONBody(errors.MethodNotAllowed, null, response, log);
responseJSONBody(errors.MethodNotAllowed, null, response, log);
return callback(errors.MethodNotAllowed);
}

if (request.method === 'GET') {
return route(request, response, userInfo, log, err => {
if (err) {
return responseJSONBody(err, null, response, log);
responseJSONBody(err, null, response, log);
}
return undefined;
return callback(err);
});
}

const op = request.query.operation;

if (!op || typeof route[op] !== 'function') {
log.error('Invalid operataion parameter', { operation: op });
return responseJSONBody(errors.BadRequest, null, response, log);
responseJSONBody(errors.BadRequest, null, response, log);
return callback(errors.BadRequest);
}

return _getRequestPayload(request, (err, payload) => {
if (err) {
return responseJSONBody(err, null, response, log);
responseJSONBody(err, null, response, log);
return callback(err);
}

let parsedIndex;
Expand All @@ -1370,20 +1344,20 @@ function routeIndexingAPIs(request, response, userInfo, log) {
parsedIndex = joi.attempt(JSON.parse(payload), indexingSchema, 'invalid payload');
} catch (err) {
log.error('Unable to parse index request body', { error: err });
return responseJSONBody(errors.BadRequest, null, response, log);
responseJSONBody(errors.BadRequest, null, response, log);
return callback(errors.BadRequest);
}

return route[op](parsedIndex, request, response, userInfo, log, err => {
if (err) {
return responseJSONBody(err, null, response, log);
responseJSONBody(err, null, response, log);
}
return undefined;
return callback(err);
});
});
}


function routeBackbeat(clientIP, request, response, log) {
function routeBackbeat(clientIP, request, response, log, callback) {
// Attach the apiMethod method to the request, so it can used by monitoring in the server
// eslint-disable-next-line no-param-reassign
request.apiMethod = 'routeBackbeat';
Expand Down Expand Up @@ -1411,14 +1385,20 @@ function routeBackbeat(clientIP, request, response, log) {
// eslint-disable-next-line no-param-reassign
request.finalizerHooks = [];

// Extract all the _apiMethods and store them in an array
const apiMethods = requestContexts ? requestContexts.map(context => context._apiMethod) : [];
// Attach the names to the current request
// eslint-disable-next-line no-param-reassign
request.apiMethods = apiMethods;

// proxy api requests to Backbeat API server
if (request.resourceType === 'api') {
if (!config.backbeat) {
log.debug('unable to proxy backbeat api request', {
backbeatConfig: config.backbeat,
});
return responseJSONBody(errors.MethodNotAllowed, null, response,
log);
responseJSONBody(errors.MethodNotAllowed, null, response, log);
return callback(errors.MethodNotAllowed);
}
const path = request.url.replace('/_/backbeat/api', '/_/');
const { host, port } = config.backbeat;
Expand All @@ -1433,10 +1413,30 @@ function routeBackbeat(clientIP, request, response, log) {
bucketName: request.bucketName,
objectKey: request.objectKey,
});
return responseJSONBody(err, null, response, log);
responseJSONBody(err, null, response, log);
return callback(err);
}
// eslint-disable-next-line no-param-reassign
request.accountQuotas = infos?.accountQuota;
if (authorizationResults) {
const checkedResults = checkAuthResults(authorizationResults, apiMethods[0], log);
if (checkedResults instanceof Error) {
responseJSONBody(errors.AccessDenied, null, response, log);
return callback(errors.AccessDenied);
}
// eslint-disable-next-line no-param-reassign
request.actionImplicitDenies = checkedResults.isImplicitDeny;
} else {
// create an object of keys apiMethods with all values to false:
// for backward compatibility, all apiMethods are allowed by default
// thus it is explicitly allowed, so implicit deny is false
// eslint-disable-next-line no-param-reassign
request.actionImplicitDenies = apiMethods.reduce((acc, curr) => {
// eslint-disable-next-line no-param-reassign
acc[curr] = false;
return acc;
}, {});
}
// FIXME for now, any authenticated user can access API
// routes. We should introduce admin accounts or accounts
// with admin privileges, and restrict access to those
Expand All @@ -1447,14 +1447,14 @@ function routeBackbeat(clientIP, request, response, log) {
bucketName: request.bucketName,
objectKey: request.objectKey,
});
return responseJSONBody(
errors.AccessDenied, null, response, log);
responseJSONBody(errors.AccessDenied, null, response, log);
return callback(errors.AccessDenied);
}
return backbeatProxy.web(request, response, { target }, err => {
log.error('error proxying request to api server',
{ error: err.message });
return responseJSONBody(errors.ServiceUnavailable, null,
response, log);
{ error: err.message });
responseJSONBody(errors.ServiceUnavailable, null, response, log);
return callback(errors.ServiceUnavailable);
});
}, 's3', requestContexts);
}
Expand Down Expand Up @@ -1485,7 +1485,8 @@ function routeBackbeat(clientIP, request, response, log) {
resourceType: request.resourceType,
query: request.query,
});
return responseJSONBody(errors.MethodNotAllowed, null, response, log);
responseJSONBody(errors.MethodNotAllowed, null, response, log);
return callback(errors.MethodNotAllowed);
}

return async.waterfall([next => auth.server.doAuth(
Expand All @@ -1501,6 +1502,24 @@ function routeBackbeat(clientIP, request, response, log) {
}
// eslint-disable-next-line no-param-reassign
request.accountQuotas = infos?.accountQuota;
if (authorizationResults) {
const checkedResults = checkAuthResults(authorizationResults, apiMethods[0], log);
if (checkedResults instanceof Error) {
return callback(errors.MethodNotAllowed);
}
// eslint-disable-next-line no-param-reassign
request.actionImplicitDenies = checkedResults.isImplicitDeny;
} else {
// create an object of keys apiMethods with all values to false:
// for backward compatibility, all apiMethods are allowed by default
// thus it is explicitly allowed, so implicit deny is false
// eslint-disable-next-line no-param-reassign
request.actionImplicitDenies = apiMethods.reduce((acc, curr) => {
// eslint-disable-next-line no-param-reassign
acc[curr] = false;
return acc;
}, {});
}
return next(err, userInfo);
}, 's3', requestContexts),
(userInfo, next) => {
Expand All @@ -1517,15 +1536,15 @@ function routeBackbeat(clientIP, request, response, log) {
}

if (request.resourceType === 'index') {
return routeIndexingAPIs(request, response, userInfo, log);
return routeIndexingAPIs(request, response, userInfo, log, callback);
}

const route = backbeatRoutes[request.method][request.resourceType];
return route(request, response, userInfo, log, err => {
if (err) {
return responseJSONBody(err, null, response, log);
responseJSONBody(err, null, response, log);
}
return undefined;
return callback(err);
});
}

Expand Down Expand Up @@ -1561,8 +1580,8 @@ function routeBackbeat(clientIP, request, response, log) {
return backbeatRoutes[request.method][request.resourceType](
request, response, log, next);
}
return backbeatRoutes[request.method][request.resourceType]
[request.query.operation](request, response, log, next);
return backbeatRoutes[request.method][request.resourceType][request.query.operation](
request, response, log, next);
}],
err => async.forEachLimit(
// Finalizer hooks are used in a quota context and ensure consistent
Expand All @@ -1573,16 +1592,20 @@ function routeBackbeat(clientIP, request, response, log) {
(hook, done) => hook(err, done),
() => {
if (err) {
return responseJSONBody(err, null, response, log);
responseJSONBody(err, null, response, log);
return callback(err);
}
log.debug('backbeat route response sent successfully',
{ method: request.method,
bucketName: request.bucketName,
objectKey: request.objectKey });
return undefined;
return callback();
},
));
}


module.exports = routeBackbeat;
module.exports = {
backbeatRoutes,
routeBackbeat,
};
2 changes: 1 addition & 1 deletion lib/utilities/internalHandlers.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const routeBackbeat = require('../routes/routeBackbeat');
const { routeBackbeat } = require('../routes/routeBackbeat');
const routeMetadata = require('../routes/routeMetadata');
const routeWorkflowEngineOperator =
require('../routes/routeWorkflowEngineOperator');
Expand Down
Loading

0 comments on commit c0bb428

Please sign in to comment.