From 8d4ff7df5f262842e753e653736c023255d091d2 Mon Sep 17 00:00:00 2001 From: Will Toozs Date: Mon, 11 Sep 2023 18:05:21 +0200 Subject: [PATCH 1/4] CLDSRV-424: api call updated with implicit deny logic change variable names for clarity edit: update arsenal package --- lib/api/api.js | 40 ++++++++++++++++++++++++++++++++++++---- package.json | 2 +- yarn.lock | 4 ++-- 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/lib/api/api.js b/lib/api/api.js index bd9e7905f7..0547ac8d45 100644 --- a/lib/api/api.js +++ b/lib/api/api.js @@ -107,6 +107,7 @@ const api = { // no need to check auth on website or cors preflight requests if (apiMethod === 'websiteGet' || apiMethod === 'websiteHead' || apiMethod === 'corsPreflight') { + request.actionImplicitDenies = false; return this[apiMethod](request, log, callback); } @@ -129,15 +130,25 @@ const api = { const requestContexts = prepareRequestContexts(apiMethod, request, sourceBucket, sourceObject, sourceVersionId); + // 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; 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) { + 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 ' + @@ -146,13 +157,25 @@ const api = { } } else { for (let i = 0; i < authResults.length; i++) { - if (!authResults[i].isAllowed) { + 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; + } else if (authResults[i].isAllowed) { + // If the action is allowed, the result is not implicit + // Deny. + isImplicitDeny[authResults[i].action] = false; + isOnlyImplicitDeny = false; } } } - return returnTagCount; + // 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([ @@ -230,7 +253,16 @@ const api = { if (checkedResults instanceof Error) { return callback(checkedResults); } - returnTagCount = checkedResults; + returnTagCount = checkedResults.returnTagCount; + 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 + request.actionImplicitDenies = apiMethods.reduce((acc, curr) => { + acc[curr] = false; + return acc; + }, {}); } if (apiMethod === 'objectPut' || apiMethod === 'objectPutPart') { request._response = response; diff --git a/package.json b/package.json index d43341009a..a0d2ab1d1d 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "homepage": "https://github.com/scality/S3#readme", "dependencies": { "@hapi/joi": "^17.1.0", - "arsenal": "git+https://github.com/scality/arsenal#7.10.48", + "arsenal": "git+https://github.com/scality/arsenal#df5ff0f4006ee0a21269e139567fd5c425a4225f", "async": "~2.5.0", "aws-sdk": "2.905.0", "azure-storage": "^2.1.0", diff --git a/yarn.lock b/yarn.lock index 2b6f432141..f9d1e14883 100644 --- a/yarn.lock +++ b/yarn.lock @@ -488,9 +488,9 @@ arraybuffer.slice@~0.0.7: optionalDependencies: ioctl "^2.0.2" -"arsenal@git+https://github.com/scality/arsenal#7.10.48": +"arsenal@git+https://github.com/scality/arsenal#df5ff0f4006ee0a21269e139567fd5c425a4225f": version "7.10.48" - resolved "git+https://github.com/scality/arsenal#f49cea3914390880008e3d41cedb1a02f9d99f39" + resolved "git+https://github.com/scality/arsenal#df5ff0f4006ee0a21269e139567fd5c425a4225f" dependencies: "@types/async" "^3.2.12" "@types/utf8" "^3.0.1" From 733f424a4bcac2f9870e89a736e39898a3467698 Mon Sep 17 00:00:00 2001 From: Maha Benzekri Date: Fri, 3 Nov 2023 10:54:03 +0100 Subject: [PATCH 2/4] CLDSRV-424:ARSN version bump --- package.json | 2 +- yarn.lock | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index a0d2ab1d1d..f11d026de5 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "homepage": "https://github.com/scality/S3#readme", "dependencies": { "@hapi/joi": "^17.1.0", - "arsenal": "git+https://github.com/scality/arsenal#df5ff0f4006ee0a21269e139567fd5c425a4225f", + "arsenal": "git+https://github.com/scality/arsenal#7.10.49", "async": "~2.5.0", "aws-sdk": "2.905.0", "azure-storage": "^2.1.0", diff --git a/yarn.lock b/yarn.lock index f9d1e14883..02cd927998 100644 --- a/yarn.lock +++ b/yarn.lock @@ -488,9 +488,9 @@ arraybuffer.slice@~0.0.7: optionalDependencies: ioctl "^2.0.2" -"arsenal@git+https://github.com/scality/arsenal#df5ff0f4006ee0a21269e139567fd5c425a4225f": - version "7.10.48" - resolved "git+https://github.com/scality/arsenal#df5ff0f4006ee0a21269e139567fd5c425a4225f" +"arsenal@git+https://github.com/scality/arsenal#7.10.49": + version "7.10.49" + resolved "git+https://github.com/scality/arsenal#fbf5562a1180055249745881c1a324562d7cdc8a" dependencies: "@types/async" "^3.2.12" "@types/utf8" "^3.0.1" From 154207581945c4227abdd442f0b102450bef5cbd Mon Sep 17 00:00:00 2001 From: Maha Benzekri Date: Fri, 3 Nov 2023 11:02:01 +0100 Subject: [PATCH 3/4] CLDSRV-424:CLDSRV version bump --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index f11d026de5..f983a22184 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "s3", - "version": "7.10.31", + "version": "7.10.32", "description": "S3 connector", "main": "index.js", "engines": { From 72fd343366b269db7994d0074bb6c93202006479 Mon Sep 17 00:00:00 2001 From: benzekrimaha <144012792+benzekrimaha@users.noreply.github.com> Date: Tue, 7 Nov 2023 09:00:24 +0100 Subject: [PATCH 4/4] Update lib/api/api.js Co-authored-by: Jonathan Gramain --- lib/api/api.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/api/api.js b/lib/api/api.js index 0547ac8d45..c326d0ac56 100644 --- a/lib/api/api.js +++ b/lib/api/api.js @@ -162,7 +162,8 @@ const api = { // Any explicit deny rejects the current API call log.trace('authorization denial from Vault'); return errors.AccessDenied; - } else if (authResults[i].isAllowed) { + } + if (authResults[i].isAllowed) { // If the action is allowed, the result is not implicit // Deny. isImplicitDeny[authResults[i].action] = false;