Skip to content

Commit 5b24bc2

Browse files
Support detecting the http scheme for policy and RQ evaluation
- Bucket policies evaluation is also updated to not compute the request context for each statement, as it will remain the same for a given request anyway. - The isSecure flag is computed every time we need to create request contexts to send to the IAM service. Issue: CLDSRV-602
1 parent 66e7dae commit 5b24bc2

File tree

8 files changed

+37
-27
lines changed

8 files changed

+37
-27
lines changed

lib/api/apiUtils/authorization/permissionChecks.js

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -255,28 +255,17 @@ function _checkBucketPolicyActions(requestType, actions, log) {
255255
return evaluators.isActionApplicable(mappedAction, actions, log);
256256
}
257257

258-
function _checkBucketPolicyResources(request, resource, log) {
259-
if (!request || (Array.isArray(resource) && resource.length === 0)) {
258+
function _checkBucketPolicyResources(requestContext, resource, log) {
259+
if (!requestContext || (Array.isArray(resource) && resource.length === 0)) {
260260
return true;
261261
}
262-
// build request context from the request!
263-
const requestContext = new RequestContext(request.headers, request.query,
264-
request.bucketName, request.objectKey, null,
265-
request.connection.encrypted, request.resourceType, 's3');
266262
return evaluators.isResourceApplicable(requestContext, resource, log);
267263
}
268264

269-
function _checkBucketPolicyConditions(request, conditions, log) {
270-
const ip = request ? requestUtils.getClientIp(request, config) : undefined;
271-
if (!conditions) {
265+
function _checkBucketPolicyConditions(requestContext, conditions, log) {
266+
if (!requestContext || !conditions) {
272267
return true;
273268
}
274-
// build request context from the request!
275-
const requestContext = new RequestContext(request.headers, request.query,
276-
request.bucketName, request.objectKey, ip,
277-
request.connection.encrypted, request.resourceType, 's3', null, null,
278-
null, null, null, null, null, null, null, null, null,
279-
request.objectLockRetentionDays);
280269
return evaluators.meetConditions(requestContext, conditions, log);
281270
}
282271

@@ -332,12 +321,22 @@ function checkBucketPolicy(policy, requestType, canonicalID, arn, bucketOwner, l
332321
permission = 'allow';
333322
}
334323
let copiedStatement = JSON.parse(JSON.stringify(policy.Statement));
324+
325+
const ip = request ? requestUtils.getClientIp(request, config) : undefined;
326+
const isSecure = request ? requestUtils.getHttpProtocolSecurity(request, config) : undefined;
327+
const requestContext = request ? new RequestContext(request.headers, request.query,
328+
request.bucketName, request.objectKey, ip,
329+
isSecure, request.resourceType, 's3', null, null,
330+
null, null, null, null, null, null, null, null, null,
331+
request.objectLockRetentionDays) : undefined;
332+
335333
while (copiedStatement.length > 0) {
336334
const s = copiedStatement[0];
335+
337336
const principalMatch = _checkPrincipals(canonicalID, arn, s.Principal);
338337
const actionMatch = _checkBucketPolicyActions(requestType, s.Action, log);
339-
const resourceMatch = _checkBucketPolicyResources(request, s.Resource, log);
340-
const conditionsMatch = _checkBucketPolicyConditions(request, s.Condition, log);
338+
const resourceMatch = _checkBucketPolicyResources(requestContext, s.Resource, log);
339+
const conditionsMatch = _checkBucketPolicyConditions(requestContext, s.Condition, log);
341340

342341
if (principalMatch && actionMatch && resourceMatch && conditionsMatch && s.Effect === 'Deny') {
343342
// explicit deny trumps any allows, so return immediately

lib/api/apiUtils/authorization/prepareRequestContexts.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,12 @@ function prepareRequestContexts(apiMethod, request, sourceBucket,
4545
// check.
4646

4747
const ip = requestUtils.getClientIp(request, config);
48+
const isSecure = requestUtils.getHttpProtocolSecurity(request, config);
4849

4950
function generateRequestContext(apiMethod) {
5051
return new RequestContext(request.headers,
5152
request.query, request.bucketName, request.objectKey,
52-
ip, request.connection.encrypted,
53-
apiMethod, 's3');
53+
ip, isSecure, apiMethod, 's3');
5454
}
5555

5656
if (apiMethod === 'bucketPut') {
@@ -84,7 +84,7 @@ function prepareRequestContexts(apiMethod, request, sourceBucket,
8484
{ versionId: sourceVersionId });
8585
const getRequestContext = new RequestContext(request.headers,
8686
reqQuery, sourceBucket, sourceObject,
87-
ip, request.connection.encrypted,
87+
ip, isSecure,
8888
objectGetAction, 's3');
8989
const putRequestContext = generateRequestContext('objectPut');
9090
requestContexts.push(getRequestContext, putRequestContext);

lib/api/apiUtils/object/objectLockHelpers.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,14 +283,15 @@ function checkUserGovernanceBypass(request, authInfo, bucketMD, objectKey, log,
283283

284284
const authParams = auth.server.extractParams(request, log, 's3', request.query);
285285
const ip = policies.requestUtils.getClientIp(request, config);
286+
const isSecure = policies.requestUtils.getHttpProtocolSecurity(request, config);
286287
const requestContextParams = {
287288
constantParams: {
288289
headers: request.headers,
289290
query: request.query,
290291
generalResource: bucketMD.getName(),
291292
specificResource: { key: objectKey },
292293
requesterIp: ip,
293-
sslEnabled: request.connection.encrypted,
294+
sslEnabled: isSecure,
294295
apiMethod: 'bypassGovernanceRetention',
295296
awsService: 's3',
296297
locationConstraint: bucketMD.getLocationConstraint(),

lib/api/bucketPut.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,8 @@ function _parseXML(request, log, cb) {
111111
});
112112
}
113113

114-
function _buildConstantParams({ request, bucketName, authInfo, authParams, ip, locationConstraint, apiMethod }) {
114+
function _buildConstantParams({
115+
request, bucketName, authInfo, authParams, ip, isSecure, locationConstraint, apiMethod }) {
115116
return {
116117
constantParams: {
117118
headers: request.headers,
@@ -121,7 +122,7 @@ function _buildConstantParams({ request, bucketName, authInfo, authParams, ip, l
121122
key: '',
122123
},
123124
requesterIp: ip,
124-
sslEnabled: request.connection.encrypted,
125+
sslEnabled: isSecure,
125126
awsService: 's3',
126127
requesterInfo: authInfo,
127128
signatureVersion: authParams.params.data.authType,
@@ -165,9 +166,11 @@ function _isAclProvided(headers) {
165166

166167
function authBucketPut(authParams, bucketName, locationConstraint, request, authInfo) {
167168
const ip = requestUtils.getClientIp(request, config);
169+
const isSecure = requestUtils.getHttpProtocolSecurity(request, config);
168170
const baseParams = {
169171
authParams,
170172
ip,
173+
isSecure,
171174
bucketName,
172175
request,
173176
authInfo,

lib/api/multiObjectDelete.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,6 @@ function getObjMetadataAndDelete(authInfo, canonicalID, request,
464464
* @param {string} request.post - concatenation of request body
465465
* @param {string} request.bucketName - parsed bucketName
466466
* @param {string} request.socket.remoteAddress - requester IP
467-
* @param {boolean} request.connection.encrypted - whether request was encrypted
468467
* @param {object} log - Werelogs logger
469468
* @param {function} callback - callback to server
470469
* @return {undefined}
@@ -487,6 +486,8 @@ function multiObjectDelete(authInfo, request, log, callback) {
487486
const inPlayInternal = [];
488487
const bucketName = request.bucketName;
489488
const canonicalID = authInfo.getCanonicalID();
489+
const ip = requestUtils.getClientIp(request, config);
490+
const isSecure = requestUtils.getHttpProtocolSecurity(request, config);
490491

491492
return async.waterfall([
492493
function parseXML(next) {
@@ -546,14 +547,13 @@ function multiObjectDelete(authInfo, request, log, callback) {
546547
// params on to this api
547548
const authParams = auth.server.extractParams(request, log,
548549
's3', request.query);
549-
const ip = requestUtils.getClientIp(request, config);
550550
const requestContextParams = {
551551
constantParams: {
552552
headers: request.headers,
553553
query: request.query,
554554
generalResource: request.bucketName,
555555
requesterIp: ip,
556-
sslEnabled: request.connection.encrypted,
556+
sslEnabled: isSecure,
557557
apiMethod: 'objectDelete',
558558
awsService: 's3',
559559
locationConstraint: null,

lib/routes/routeMetadata.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,10 @@ function routeMetadata(clientIP, request, response, log) {
5050
return responseJSONBody(errors.NotImplemented, null, response, log);
5151
}
5252
const ip = requestUtils.getClientIp(request, config);
53+
const isSecure = requestUtils.getHttpProtocolSecurity(request, config);
5354
const requestContexts = [new RequestContext(request.headers, request.query,
5455
request.generalResource, request.specificResource, ip,
55-
request.connection.encrypted, request.resourceType, 'metadata')];
56+
isSecure, request.resourceType, 'metadata')];
5657
return waterfall([
5758
next => auth.server.doAuth(request, log, (err, userInfo, authRes) => {
5859
if (err) {

tests/unit/api/deleteMarker.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@ function _createMultiObjectDeleteRequest(numObjects) {
6464
},
6565
url: '/?delete',
6666
query: { delete: '' },
67+
socket: {
68+
remoteAddress: '127.0.0.1',
69+
},
6770
actionImplicitDenies: false,
6871
};
6972
const xml = [];

tests/unit/api/multiObjectDelete.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,9 @@ describe('multiObjectDelete function', () => {
380380
'content-md5': crypto.createHash('md5').update(post, 'utf8').digest('base64')
381381
},
382382
post,
383+
socket: {
384+
remoteAddress: '127.0.0.1',
385+
},
383386
url: '/bucketname',
384387
});
385388
const authInfo = makeAuthInfo('123456');

0 commit comments

Comments
 (0)