Skip to content

Support SSL condition #5724

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 16 additions & 17 deletions lib/api/apiUtils/authorization/permissionChecks.js
Original file line number Diff line number Diff line change
Expand Up @@ -255,28 +255,17 @@ function _checkBucketPolicyActions(requestType, actions, log) {
return evaluators.isActionApplicable(mappedAction, actions, log);
}

function _checkBucketPolicyResources(request, resource, log) {
if (!request || (Array.isArray(resource) && resource.length === 0)) {
function _checkBucketPolicyResources(requestContext, resource, log) {
if (!requestContext || (Array.isArray(resource) && resource.length === 0)) {
return true;
}
// build request context from the request!
const requestContext = new RequestContext(request.headers, request.query,
request.bucketName, request.objectKey, null,
request.connection.encrypted, request.resourceType, 's3');
return evaluators.isResourceApplicable(requestContext, resource, log);
}

function _checkBucketPolicyConditions(request, conditions, log) {
const ip = request ? requestUtils.getClientIp(request, config) : undefined;
if (!conditions) {
function _checkBucketPolicyConditions(requestContext, conditions, log) {
if (!requestContext || !conditions) {
return true;
}
// build request context from the request!
const requestContext = new RequestContext(request.headers, request.query,
request.bucketName, request.objectKey, ip,
request.connection.encrypted, request.resourceType, 's3', null, null,
null, null, null, null, null, null, null, null, null,
request.objectLockRetentionDays);
return evaluators.meetConditions(requestContext, conditions, log);
}

Expand Down Expand Up @@ -332,12 +321,22 @@ function checkBucketPolicy(policy, requestType, canonicalID, arn, bucketOwner, l
permission = 'allow';
}
let copiedStatement = JSON.parse(JSON.stringify(policy.Statement));

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,
request.bucketName, request.objectKey, ip,
isSecure, request.resourceType, 's3', null, null,
null, null, null, null, null, null, null, null, null,
request.objectLockRetentionDays) : undefined;

while (copiedStatement.length > 0) {
const s = copiedStatement[0];

const principalMatch = _checkPrincipals(canonicalID, arn, s.Principal);
const actionMatch = _checkBucketPolicyActions(requestType, s.Action, log);
const resourceMatch = _checkBucketPolicyResources(request, s.Resource, log);
const conditionsMatch = _checkBucketPolicyConditions(request, s.Condition, log);
const resourceMatch = _checkBucketPolicyResources(requestContext, s.Resource, log);
const conditionsMatch = _checkBucketPolicyConditions(requestContext, s.Condition, log);

if (principalMatch && actionMatch && resourceMatch && conditionsMatch && s.Effect === 'Deny') {
// explicit deny trumps any allows, so return immediately
Expand Down
6 changes: 3 additions & 3 deletions lib/api/apiUtils/authorization/prepareRequestContexts.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ function prepareRequestContexts(apiMethod, request, sourceBucket,
// check.

const ip = requestUtils.getClientIp(request, config);
const isSecure = requestUtils.getHttpProtocolSecurity(request, config);

function generateRequestContext(apiMethod) {
return new RequestContext(request.headers,
request.query, request.bucketName, request.objectKey,
ip, request.connection.encrypted,
apiMethod, 's3');
ip, isSecure, apiMethod, 's3');
}

if (apiMethod === 'bucketPut') {
Expand Down Expand Up @@ -84,7 +84,7 @@ function prepareRequestContexts(apiMethod, request, sourceBucket,
{ versionId: sourceVersionId });
const getRequestContext = new RequestContext(request.headers,
reqQuery, sourceBucket, sourceObject,
ip, request.connection.encrypted,
ip, isSecure,
objectGetAction, 's3');
const putRequestContext = generateRequestContext('objectPut');
requestContexts.push(getRequestContext, putRequestContext);
Expand Down
3 changes: 2 additions & 1 deletion lib/api/apiUtils/object/objectLockHelpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -283,14 +283,15 @@

const authParams = auth.server.extractParams(request, log, 's3', request.query);
const ip = policies.requestUtils.getClientIp(request, config);
const isSecure = policies.requestUtils.getHttpProtocolSecurity(request, config);

Check warning on line 286 in lib/api/apiUtils/object/objectLockHelpers.js

View check run for this annotation

Codecov / codecov/patch

lib/api/apiUtils/object/objectLockHelpers.js#L286

Added line #L286 was not covered by tests
const requestContextParams = {
constantParams: {
headers: request.headers,
query: request.query,
generalResource: bucketMD.getName(),
specificResource: { key: objectKey },
requesterIp: ip,
sslEnabled: request.connection.encrypted,
sslEnabled: isSecure,
apiMethod: 'bypassGovernanceRetention',
awsService: 's3',
locationConstraint: bucketMD.getLocationConstraint(),
Expand Down
7 changes: 5 additions & 2 deletions lib/api/bucketPut.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ function _parseXML(request, log, cb) {
});
}

function _buildConstantParams({ request, bucketName, authInfo, authParams, ip, locationConstraint, apiMethod }) {
function _buildConstantParams({
request, bucketName, authInfo, authParams, ip, isSecure, locationConstraint, apiMethod }) {
return {
constantParams: {
headers: request.headers,
Expand All @@ -121,7 +122,7 @@ function _buildConstantParams({ request, bucketName, authInfo, authParams, ip, l
key: '',
},
requesterIp: ip,
sslEnabled: request.connection.encrypted,
sslEnabled: isSecure,
awsService: 's3',
requesterInfo: authInfo,
signatureVersion: authParams.params.data.authType,
Expand Down Expand Up @@ -165,9 +166,11 @@ function _isAclProvided(headers) {

function authBucketPut(authParams, bucketName, locationConstraint, request, authInfo) {
const ip = requestUtils.getClientIp(request, config);
const isSecure = requestUtils.getHttpProtocolSecurity(request, config);
const baseParams = {
authParams,
ip,
isSecure,
bucketName,
request,
authInfo,
Expand Down
6 changes: 3 additions & 3 deletions lib/api/multiObjectDelete.js
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,6 @@ function getObjMetadataAndDelete(authInfo, canonicalID, request,
* @param {string} request.post - concatenation of request body
* @param {string} request.bucketName - parsed bucketName
* @param {string} request.socket.remoteAddress - requester IP
* @param {boolean} request.connection.encrypted - whether request was encrypted
* @param {object} log - Werelogs logger
* @param {function} callback - callback to server
* @return {undefined}
Expand All @@ -487,6 +486,8 @@ function multiObjectDelete(authInfo, request, log, callback) {
const inPlayInternal = [];
const bucketName = request.bucketName;
const canonicalID = authInfo.getCanonicalID();
const ip = requestUtils.getClientIp(request, config);
const isSecure = requestUtils.getHttpProtocolSecurity(request, config);

return async.waterfall([
function parseXML(next) {
Expand Down Expand Up @@ -546,14 +547,13 @@ function multiObjectDelete(authInfo, request, log, callback) {
// params on to this api
const authParams = auth.server.extractParams(request, log,
's3', request.query);
const ip = requestUtils.getClientIp(request, config);
const requestContextParams = {
constantParams: {
headers: request.headers,
query: request.query,
generalResource: request.bucketName,
requesterIp: ip,
sslEnabled: request.connection.encrypted,
sslEnabled: isSecure,
apiMethod: 'objectDelete',
awsService: 's3',
locationConstraint: null,
Expand Down
3 changes: 2 additions & 1 deletion lib/routes/routeMetadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,10 @@
return responseJSONBody(errors.NotImplemented, null, response, log);
}
const ip = requestUtils.getClientIp(request, config);
const isSecure = requestUtils.getHttpProtocolSecurity(request, config);

Check warning on line 53 in lib/routes/routeMetadata.js

View check run for this annotation

Codecov / codecov/patch

lib/routes/routeMetadata.js#L53

Added line #L53 was not covered by tests
const requestContexts = [new RequestContext(request.headers, request.query,
request.generalResource, request.specificResource, ip,
request.connection.encrypted, request.resourceType, 'metadata')];
isSecure, request.resourceType, 'metadata')];
return waterfall([
next => auth.server.doAuth(request, log, (err, userInfo, authRes) => {
if (err) {
Expand Down
3 changes: 3 additions & 0 deletions tests/unit/api/deleteMarker.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ function _createMultiObjectDeleteRequest(numObjects) {
},
url: '/?delete',
query: { delete: '' },
socket: {
remoteAddress: '127.0.0.1',
},
actionImplicitDenies: false,
};
const xml = [];
Expand Down
3 changes: 3 additions & 0 deletions tests/unit/api/multiObjectDelete.js
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,9 @@ describe('multiObjectDelete function', () => {
'content-md5': crypto.createHash('md5').update(post, 'utf8').digest('base64')
},
post,
socket: {
remoteAddress: '127.0.0.1',
},
url: '/bucketname',
});
const authInfo = makeAuthInfo('123456');
Expand Down
Loading