Compare commits

...

1 Commits

Author SHA1 Message Date
Alexander Chan 3d4273ca61 S3C-2663: fix missing object access response with bucket policy 2021-05-17 15:43:32 -07:00
4 changed files with 411 additions and 50 deletions

View File

@ -317,10 +317,87 @@ function isBucketAuthorized(bucket, requestType, canonicalID, authInfo, log) {
return (aclPermission || (bucketPolicyPermission === 'allow'));
}
function _isPermissionGranted(permissionList, canonicalID) {
if (permissionList.indexOf(publicId) > -1) {
return true;
}
if (permissionList.indexOf(allAuthedUsersId) > -1 &&
canonicalID !== publicId) {
return true;
}
if (permissionList.indexOf(canonicalID) > -1) {
return true;
}
return false;
}
function hasBucketReadAccess(bucket, requestType, canonicalID, authInfo, log) {
const bucketAcl = bucket.getAcl();
const bucketOwner = bucket.getOwner();
const bucketPolicy = bucket.getBucketPolicy();
let arn = null;
let requesterIsNotUser = true;
// User is already authorized on the bucket for FULL_CONTROL or WRITE or
// bucket has canned ACL public-read-write
if (requestType === 'objectPut' || requestType === 'objectDelete') {
return true;
}
if (authInfo) {
requesterIsNotUser = !authInfo.isRequesterAnIAMUser();
arn = authInfo.getArn();
}
// bucket policies over acls if any is applicable
if (bucketPolicy) {
// bucketGet covers listObjects and listMultipartUploads, bucket read
// actions
const bucketListPermission = checkBucketPolicy(
bucketPolicy,
'bucketGet',
canonicalID,
arn,
bucketOwner,
log
);
if (bucketListPermission === 'explicitDeny') {
return false;
}
if (bucketListPermission === 'allow') {
return true;
}
// defaultDeny, fallback onto acls
}
if ((canonicalID === bucketOwner && requesterIsNotUser) ||
_isPermissionGranted(bucketAcl.FULL_CONTROL, canonicalID) ||
_isPermissionGranted(bucketAcl.READ, canonicalID)) {
return true;
}
if (bucketAcl.Canned === 'public-read' ||
bucketAcl.Canned === 'public-read-write' ||
(bucketAcl.Canned === 'authenticated-read' &&
canonicalID !== publicId)) {
return true;
}
return false;
}
function isObjAuthorized(bucket, objectMD, requestType, canonicalID, authInfo, log) {
const bucketOwner = bucket.getOwner();
if (!objectMD) {
return false;
// if read access is granted, return true to have the api handler
// respond accordingly to the missing object metadata
// if read access is not granted, return false for AccessDenied
return hasBucketReadAccess(bucket, requestType, canonicalID, authInfo, log);
}
let requesterIsNotUser = true;
let arn = null;

View File

@ -203,11 +203,7 @@ function metadataValidateBucketAndObj(params, log, callback) {
return next(null, bucket, objMD);
},
function checkObjectAuth(bucket, objMD, next) {
if (!objMD) {
return next(null, bucket);
}
if (!isObjAuthorized(bucket, objMD, requestType, canonicalID,
authInfo, log)) {
if (!isObjAuthorized(bucket, objMD, requestType, canonicalID, authInfo, log)) {
log.debug('access denied for user on object', { requestType });
return next(errors.AccessDenied, bucket);
}

View File

@ -275,3 +275,188 @@ describe('object authorization for objectPutACL and objectGetACL', () => {
assert.strictEqual(authorizedResult, true);
});
});
describe('without object metadata', () => {
afterEach(() => {
bucket.setFullAcl({
Canned: 'private',
FULL_CONTROL: [],
WRITE: [],
WRITE_ACP: [],
READ: [],
READ_ACP: [],
});
bucket.setBucketPolicy(null);
});
const requestTypes = [
'objectGet',
'objectHead',
'objectPutACL',
'objectGetACL',
];
const allowedAccess = [true, true, true, true];
const deniedAccess = [false, false, false, false];
const tests = [
{
it: 'should allow bucket owner',
canned: 'private', id: bucketOwnerCanonicalId,
aclParam: null,
response: allowedAccess,
},
{
it: 'should not allow public if canned private',
canned: 'private', id: constants.publicId,
aclParam: null,
response: deniedAccess,
},
{
it: 'should not other accounts if canned private',
canned: 'private', id: accountToVet,
aclParam: null,
response: deniedAccess,
},
{
it: 'should allow public if bucket is canned public-read',
canned: 'public-read', id: constants.publicId,
aclParam: null,
response: allowedAccess,
},
{
it: 'should allow public if bucket is canned public-read-write',
canned: 'public-read-write', id: constants.publicId,
aclParam: null,
response: allowedAccess,
},
{
it: 'should not allow public if bucket is canned ' +
'authenticated-read',
canned: 'authenticated-read', id: constants.publicId,
aclParam: null,
response: deniedAccess,
},
{
it: 'should allow authenticated users if bucket is canned ' +
'authenticated-read',
canned: 'authenticated-read', id: accountToVet,
aclParam: null,
response: allowedAccess,
},
{
it: 'should allow account if granted bucket READ',
canned: '', id: accountToVet,
aclParam: ['READ', accountToVet],
response: allowedAccess,
},
{
it: 'should allow account if granted bucket FULL_CONTROL',
canned: '', id: accountToVet,
aclParam: ['FULL_CONTROL', accountToVet],
response: allowedAccess,
},
{
it: 'should allow public if granted bucket read action in policy',
canned: 'private', id: constants.publicId,
aclParam: null,
policy: {
Version: '2012-10-17',
Statement: [
{
Effect: 'Allow',
Resource: 'arn:aws:s3:::niftybucket',
Principal: '*',
Action: ['s3:ListBucket'],
},
],
},
response: allowedAccess,
},
{
it: 'should not allow public if denied bucket read action in policy',
canned: 'public-read', id: constants.publicId,
aclParam: null,
policy: {
Version: '2012-10-17',
Statement: [
{
Effect: 'Deny',
Resource: 'arn:aws:s3:::niftybucket',
Principal: '*',
Action: ['s3:ListBucket'],
},
],
},
response: deniedAccess,
},
{
it: 'should allow account if granted bucket read action in policy',
canned: 'private', id: accountToVet,
aclParam: null,
policy: {
Version: '2012-10-17',
Statement: [
{
Effect: 'Allow',
Resource: 'arn:aws:s3:::niftybucket',
Principal: { AWS: [altAcctAuthInfo.getArn()] },
Action: ['s3:ListBucket'],
},
],
},
response: allowedAccess,
authInfo: altAcctAuthInfo,
},
{
it: 'should not allow account if denied bucket read action in policy',
canned: 'public-read', id: accountToVet,
aclParam: null,
policy: {
Version: '2012-10-17',
Statement: [
{
Effect: 'Deny',
Resource: 'arn:aws:s3:::niftybucket',
Principal: { CanonicalUser: [altAcctAuthInfo.getCanonicalID()] },
Action: ['s3:ListBucket'],
},
],
},
response: deniedAccess,
authInfo: altAcctAuthInfo,
},
];
tests.forEach(value => {
it(value.it, done => {
const authInfoUser = value.authInfo ? value.authInfo : authInfo;
if (value.aclParam) {
bucket.setSpecificAcl(value.aclParam[1], value.aclParam[0]);
}
if (value.policy) {
bucket.setBucketPolicy(value.policy);
}
bucket.setCannedAcl(value.canned);
const results = requestTypes.map(type =>
isObjAuthorized(bucket, null, type, value.id, authInfoUser, log));
assert.deepStrictEqual(results, value.response);
done();
});
});
it('should allow access to anyone since checks ' +
'are done at bucket level', () => {
const requestTypes = ['objectPut', 'objectDelete'];
const results = requestTypes.map(type =>
isObjAuthorized(bucket, null, type, accountToVet, authInfo, log));
assert.deepStrictEqual(results, [true, true]);
const publicUserResults = requestTypes.map(type =>
isObjAuthorized(bucket, null, type, constants.publicId, authInfo, log));
assert.deepStrictEqual(publicUserResults, [true, true]);
});
});

191
yarn.lock
View File

@ -672,7 +672,6 @@ arraybuffer.slice@~0.0.7:
level "~5.0.1"
level-sublevel "~6.6.5"
node-forge "^0.7.1"
prom-client "10.2.3"
simple-glob "^0.2"
socket.io "~2.3.0"
socket.io-client "~2.3.0"
@ -734,41 +733,6 @@ arsenal@scality/Arsenal#65966f5:
optionalDependencies:
ioctl "2.0.0"
arsenal@scality/Arsenal#8ed8478:
version "8.2.1"
resolved "https://codeload.github.com/scality/Arsenal/tar.gz/8ed84786fce31f603b1e8cd641b3b44b8f715b0b"
dependencies:
"@hapi/joi" "^15.1.0"
JSONStream "^1.0.0"
ajv "6.12.2"
async "~2.6.1"
aws-sdk "2.80.0"
azure-storage "^2.1.0"
backo "^1.1.0"
bson "4.0.0"
debug "~4.1.0"
diskusage "^1.1.1"
fcntl "github:scality/node-fcntl"
hdclient scality/hdclient#5145e04e5ed33e85106765b1caa90cd245ef482b
https-proxy-agent "^2.2.0"
ioredis "4.9.5"
ipaddr.js "1.9.1"
level "~5.0.1"
level-sublevel "~6.6.5"
mongodb "^3.0.1"
node-forge "^0.7.1"
prom-client "10.2.3"
simple-glob "^0.2.0"
socket.io "~2.3.0"
socket.io-client "~2.3.0"
sproxydclient "github:scality/sproxydclient#30e7115"
utf8 "3.0.0"
uuid "^3.0.1"
werelogs scality/werelogs#0ff7ec82
xml2js "~0.4.23"
optionalDependencies:
ioctl "2.0.1"
arsenal@scality/Arsenal#9f2e74e:
version "7.4.3"
resolved "https://codeload.github.com/scality/Arsenal/tar.gz/9f2e74ec6972527c2a9ca6ecb4155618f123fc19"
@ -794,6 +758,41 @@ arsenal@scality/Arsenal#9f2e74e:
optionalDependencies:
ioctl "2.0.0"
arsenal@scality/Arsenal#c57cde8:
version "8.1.4"
resolved "https://codeload.github.com/scality/Arsenal/tar.gz/c57cde88bb04fe9803ec08c3a883f6eb986e4149"
dependencies:
JSONStream "^1.0.0"
ajv "4.10.0"
async "~2.6.1"
aws-sdk "2.80.0"
azure-storage "^2.1.0"
backo "^1.1.0"
bson "4.0.0"
debug "~4.1.0"
diskusage "^1.1.1"
fcntl "github:scality/node-fcntl"
hdclient scality/hdclient#5145e04e5ed33e85106765b1caa90cd245ef482b
https-proxy-agent "^2.2.0"
ioredis "4.9.5"
ipaddr.js "1.8.1"
joi "^14.3.0"
level "~5.0.1"
level-sublevel "~6.6.5"
mongodb "^3.0.1"
node-forge "^0.7.1"
prom-client "10.2.3"
simple-glob "^0.2.0"
socket.io "~2.2.0"
socket.io-client "~2.2.0"
sproxydclient "github:scality/sproxydclient#a6ec980"
utf8 "3.0.0"
uuid "^3.0.1"
werelogs scality/werelogs#0ff7ec82
xml2js "~0.4.16"
optionalDependencies:
ioctl "2.0.1"
arsenal@scality/Arsenal#ed446c5:
version "7.7.0"
resolved "https://codeload.github.com/scality/Arsenal/tar.gz/ed446c569ce11cd8c109c43c633de2dcd201f8a1"
@ -1212,9 +1211,9 @@ bucketclient@scality/bucketclient:
version "8.1.0"
resolved "https://codeload.github.com/scality/bucketclient/tar.gz/07d5a3813878b2746ed00e41c177bc2a0460e243"
dependencies:
agentkeepalive "^4.1.3"
arsenal scality/Arsenal#8ed8478
arsenal scality/Arsenal#c57cde8
werelogs scality/werelogs#351a2a3
yarn "^1.17.3"
bucketclient@scality/bucketclient#6d2d5a4:
version "7.4.5"
@ -2056,6 +2055,23 @@ engine.io-client@~1.8.4:
xmlhttprequest-ssl "1.5.3"
yeast "0.1.2"
engine.io-client@~3.3.1:
version "3.3.2"
resolved "https://registry.yarnpkg.com/engine.io-client/-/engine.io-client-3.3.2.tgz#04e068798d75beda14375a264bb3d742d7bc33aa"
integrity sha512-y0CPINnhMvPuwtqXfsGuWE8BB66+B6wTtCofQDRecMQPYX3MYUZXFNKDhdrSe3EVjgOu4V3rxdeqN/Tr91IgbQ==
dependencies:
component-emitter "1.2.1"
component-inherit "0.0.3"
debug "~3.1.0"
engine.io-parser "~2.1.1"
has-cors "1.1.0"
indexof "0.0.1"
parseqs "0.0.5"
parseuri "0.0.5"
ws "~6.1.0"
xmlhttprequest-ssl "~1.5.4"
yeast "0.1.2"
engine.io-client@~3.4.0:
version "3.4.4"
resolved "https://registry.yarnpkg.com/engine.io-client/-/engine.io-client-3.4.4.tgz#77d8003f502b0782dd792b073a4d2cf7ca5ab967"
@ -2085,6 +2101,17 @@ engine.io-parser@1.3.2:
has-binary "0.1.7"
wtf-8 "1.0.0"
engine.io-parser@~2.1.0, engine.io-parser@~2.1.1:
version "2.1.3"
resolved "https://registry.yarnpkg.com/engine.io-parser/-/engine.io-parser-2.1.3.tgz#757ab970fbf2dfb32c7b74b033216d5739ef79a6"
integrity sha512-6HXPre2O4Houl7c4g7Ic/XzPnHBvaEmN90vtRO9uLmwtRqQmTOw0QMevL1TOfL2Cpu1VzsaTmMotQgMdkzGkVA==
dependencies:
after "0.8.2"
arraybuffer.slice "~0.0.7"
base64-arraybuffer "0.1.5"
blob "0.0.5"
has-binary2 "~1.0.2"
engine.io-parser@~2.2.0:
version "2.2.1"
resolved "https://registry.yarnpkg.com/engine.io-parser/-/engine.io-parser-2.2.1.tgz#57ce5611d9370ee94f99641b589f94c97e4f5da7"
@ -2108,6 +2135,18 @@ engine.io@~1.8.4:
engine.io-parser "1.3.2"
ws "~1.1.5"
engine.io@~3.3.1:
version "3.3.2"
resolved "https://registry.yarnpkg.com/engine.io/-/engine.io-3.3.2.tgz#18cbc8b6f36e9461c5c0f81df2b830de16058a59"
integrity sha512-AsaA9KG7cWPXWHp5FvHdDWY3AMWeZ8x+2pUVLcn71qE5AtAzgGbxuclOytygskw8XGmiQafTmnI9Bix3uihu2w==
dependencies:
accepts "~1.3.4"
base64id "1.0.0"
cookie "0.3.1"
debug "~3.1.0"
engine.io-parser "~2.1.0"
ws "~6.1.0"
engine.io@~3.4.0:
version "3.4.2"
resolved "https://registry.yarnpkg.com/engine.io/-/engine.io-3.4.2.tgz#8fc84ee00388e3e228645e0a7d3dfaeed5bd122c"
@ -3034,6 +3073,11 @@ hoek@4.x.x:
resolved "https://registry.yarnpkg.com/hoek/-/hoek-4.2.1.tgz#9634502aa12c445dd5a7c5734b572bb8738aacbb"
integrity sha512-QLg82fGkfnJ/4iy1xZ81/9SIJiq1NGFUMGs6ParyjBZr6jW2Ufj/snDqTHixNlHdPNwN2RLVD0Pi3igeK9+JfA==
hoek@6.x.x:
version "6.1.3"
resolved "https://registry.yarnpkg.com/hoek/-/hoek-6.1.3.tgz#73b7d33952e01fe27a38b0457294b79dd8da242c"
integrity sha512-YXXAAhmF9zpQbC7LEcREFtXfGq5K1fmd+4PHkBq8NUqmzW3G+Dq10bI/i0KucLRwss3YYFQ0fSfoxBZYiGUqtQ==
hosted-git-info@^2.1.4, hosted-git-info@^2.7.1:
version "2.8.8"
resolved "https://registry.yarnpkg.com/hosted-git-info/-/hosted-git-info-2.8.8.tgz#7539bd4bc1e0e0a895815a2e0262420b12858488"
@ -3312,6 +3356,11 @@ ipaddr.js@1.2.0:
resolved "https://registry.yarnpkg.com/ipaddr.js/-/ipaddr.js-1.2.0.tgz#8aba49c9192799585bdd643e0ccb50e8ae777ba4"
integrity sha1-irpJyRknmVhb3WQ+DMtQ6K53e6Q=
ipaddr.js@1.8.1:
version "1.8.1"
resolved "https://registry.yarnpkg.com/ipaddr.js/-/ipaddr.js-1.8.1.tgz#fa4b79fa47fd3def5e3b159825161c0a519c9427"
integrity sha1-+kt5+kf9Pe9eOxWYJRYcClGclCc=
ipaddr.js@1.9.1:
version "1.9.1"
resolved "https://registry.yarnpkg.com/ipaddr.js/-/ipaddr.js-1.9.1.tgz#bff38543eeb8984825079ff3a2a8e6cbd46781b3"
@ -3556,6 +3605,13 @@ isemail@2.x.x:
resolved "https://registry.yarnpkg.com/isemail/-/isemail-2.2.1.tgz#0353d3d9a62951080c262c2aa0a42b8ea8e9e2a6"
integrity sha1-A1PT2aYpUQgMJiwqoKQrjqjp4qY=
isemail@3.x.x:
version "3.2.0"
resolved "https://registry.yarnpkg.com/isemail/-/isemail-3.2.0.tgz#59310a021931a9fb06bbb51e155ce0b3f236832c"
integrity sha512-zKqkK+O+dGqevc93KNsbZ/TqTUFd46MwWjYOoMrjIMZ51eU7DtQG3Wmd9SQQT7i7RVnuTPEiYEWHU3MSbxC1Tg==
dependencies:
punycode "2.x.x"
isexe@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/isexe/-/isexe-2.0.0.tgz#e8fbf374dc556ff8947a10dcb0572d633f2cfa10"
@ -3695,6 +3751,15 @@ joi@^10.6:
items "2.x.x"
topo "2.x.x"
joi@^14.3.0:
version "14.3.1"
resolved "https://registry.yarnpkg.com/joi/-/joi-14.3.1.tgz#164a262ec0b855466e0c35eea2a885ae8b6c703c"
integrity sha512-LQDdM+pkOrpAn4Lp+neNIFV3axv1Vna3j38bisbQhETPMANYRbFJFUyOZcOClYvM/hppMhGWuKSFEK9vjrB+bQ==
dependencies:
hoek "6.x.x"
isemail "3.x.x"
topo "3.x.x"
"js-tokens@^3.0.0 || ^4.0.0":
version "4.0.0"
resolved "https://registry.yarnpkg.com/js-tokens/-/js-tokens-4.0.0.tgz#19203fb59991df98e3a287050d4647cdeaf32499"
@ -5335,7 +5400,7 @@ punycode@1.3.2:
resolved "https://registry.yarnpkg.com/punycode/-/punycode-1.3.2.tgz#9653a036fb7c1ee42342f2325cceefea3926c48d"
integrity sha1-llOgNvt8HuQjQvIyXM7v6jkmxI0=
punycode@^2.1.0, punycode@^2.1.1:
punycode@2.x.x, punycode@^2.1.0, punycode@^2.1.1:
version "2.1.1"
resolved "https://registry.yarnpkg.com/punycode/-/punycode-2.1.1.tgz#b58b010ac40c22c5657616c8d2c2c02c7bf479ec"
integrity sha512-XRsRjdf+j5ml+y/6GKHPZbrF/8p2Yga0JPtdqTIY2Xe5ohJPD9saDJJLPvp9+NSBprVvevdXZybnj2cv8OEd0A==
@ -6218,6 +6283,26 @@ socket.io-client@1.7.4, socket.io-client@~1.7.3:
socket.io-parser "2.3.1"
to-array "0.1.4"
socket.io-client@2.2.0, socket.io-client@~2.2.0:
version "2.2.0"
resolved "https://registry.yarnpkg.com/socket.io-client/-/socket.io-client-2.2.0.tgz#84e73ee3c43d5020ccc1a258faeeb9aec2723af7"
integrity sha512-56ZrkTDbdTLmBIyfFYesgOxsjcLnwAKoN4CiPyTVkMQj3zTUh0QAx3GbvIvLpFEOvQWu92yyWICxB0u7wkVbYA==
dependencies:
backo2 "1.0.2"
base64-arraybuffer "0.1.5"
component-bind "1.0.0"
component-emitter "1.2.1"
debug "~3.1.0"
engine.io-client "~3.3.1"
has-binary2 "~1.0.2"
has-cors "1.1.0"
indexof "0.0.1"
object-component "0.0.3"
parseqs "0.0.5"
parseuri "0.0.5"
socket.io-parser "~3.3.0"
to-array "0.1.4"
socket.io-client@2.3.0:
version "2.3.0"
resolved "https://registry.yarnpkg.com/socket.io-client/-/socket.io-client-2.3.0.tgz#14d5ba2e00b9bcd145ae443ab96b3f86cbcc1bb4"
@ -6296,6 +6381,18 @@ socket.io@~1.7.3:
socket.io-client "1.7.4"
socket.io-parser "2.3.1"
socket.io@~2.2.0:
version "2.2.0"
resolved "https://registry.yarnpkg.com/socket.io/-/socket.io-2.2.0.tgz#f0f633161ef6712c972b307598ecd08c9b1b4d5b"
integrity sha512-wxXrIuZ8AILcn+f1B4ez4hJTPG24iNgxBBDaJfT6MsyOhVYiTXWexGoPkd87ktJG8kQEcL/NBvRi64+9k4Kc0w==
dependencies:
debug "~4.1.0"
engine.io "~3.3.1"
has-binary2 "~1.0.2"
socket.io-adapter "~1.1.0"
socket.io-client "2.2.0"
socket.io-parser "~3.3.0"
socket.io@~2.3.0:
version "2.3.0"
resolved "https://registry.yarnpkg.com/socket.io/-/socket.io-2.3.0.tgz#cd762ed6a4faeca59bc1f3e243c0969311eb73fb"
@ -6390,12 +6487,11 @@ sprintf-js@~1.0.2:
resolved "https://registry.yarnpkg.com/sprintf-js/-/sprintf-js-1.0.3.tgz#04e6926f662895354f3dd015203633b857297e2c"
integrity sha1-BOaSb2YolTVPPdAVIDYzuFcpfiw=
"sproxydclient@github:scality/sproxydclient#30e7115":
version "8.0.2"
resolved "https://codeload.github.com/scality/sproxydclient/tar.gz/30e7115668bc7e10b4ec3cfdbaa7a124cdc21cc5"
"sproxydclient@github:scality/sproxydclient#a6ec980":
version "7.4.0"
resolved "https://codeload.github.com/scality/sproxydclient/tar.gz/a6ec98079fcbfde113de3f3afdcb57835d2ac55f"
dependencies:
async "^3.1.0"
werelogs scality/werelogs#351a2a3
werelogs scality/werelogs#0ff7ec82
sproxydclient@scality/sproxydclient#44f025b:
version "7.4.7"
@ -6791,6 +6887,13 @@ topo@2.x.x:
dependencies:
hoek "4.x.x"
topo@3.x.x:
version "3.0.3"
resolved "https://registry.yarnpkg.com/topo/-/topo-3.0.3.tgz#d5a67fb2e69307ebeeb08402ec2a2a6f5f7ad95c"
integrity sha512-IgpPtvD4kjrJ7CRA3ov2FhWQADwv+Tdqbsf1ZnPUSAtCJ9e1Z44MmoSGDXGk4IppoZA7jd/QRkNddlLJWlUZsQ==
dependencies:
hoek "6.x.x"
tough-cookie@~2.5.0:
version "2.5.0"
resolved "https://registry.yarnpkg.com/tough-cookie/-/tough-cookie-2.5.0.tgz#cd9fb2a0aa1d5a12b473bd9fb96fa3dcff65ade2"