Skip to content

Commit 645c917

Browse files
tangledbytestangledbytes
committed
CRUD PublicAccessBlock working
Signed-off-by: Utkarsh Srivastava <[email protected]> Complete BlockPublicAccess integration Signed-off-by: Utkarsh Srivastava <[email protected]> add NC test for PublicAccessBlock Signed-off-by: Utkarsh Srivastava <[email protected]> dont check for disallow_public_access in DENY statements Signed-off-by: Utkarsh Srivastava <[email protected]>
1 parent 3f0a416 commit 645c917

19 files changed

+680
-13
lines changed

Diff for: ‎src/api/bucket_api.js

+59
Original file line numberDiff line numberDiff line change
@@ -949,6 +949,65 @@ module.exports = {
949949
system: ['admin', 'user']
950950
}
951951
},
952+
953+
get_public_access_block: {
954+
method: 'GET',
955+
params: {
956+
type: 'object',
957+
required: ['bucket_name'],
958+
properties: {
959+
bucket_name: {
960+
$ref: 'common_api#/definitions/bucket_name'
961+
}
962+
}
963+
},
964+
reply: {
965+
type: 'object',
966+
properties: {
967+
public_access_block: {
968+
$ref: 'common_api#/definitions/public_access_block'
969+
}
970+
}
971+
},
972+
auth: {
973+
system: ['admin', 'user']
974+
}
975+
},
976+
977+
put_public_access_block: {
978+
method: 'PUT',
979+
params: {
980+
type: 'object',
981+
required: ['bucket_name', 'public_access_block'],
982+
properties: {
983+
bucket_name: {
984+
$ref: 'common_api#/definitions/bucket_name'
985+
},
986+
public_access_block: {
987+
$ref: 'common_api#/definitions/public_access_block'
988+
},
989+
},
990+
},
991+
auth: {
992+
system: ['admin', 'user']
993+
}
994+
},
995+
996+
delete_public_access_block: {
997+
method: 'DELETE',
998+
params: {
999+
type: 'object',
1000+
required: ['bucket_name'],
1001+
properties: {
1002+
bucket_name: {
1003+
$ref: 'common_api#/definitions/bucket_name'
1004+
},
1005+
},
1006+
},
1007+
auth: {
1008+
system: ['admin', 'user']
1009+
}
1010+
},
9521011
},
9531012

9541013
definitions: {

Diff for: ‎src/api/common_api.js

+9
Original file line numberDiff line numberDiff line change
@@ -1483,6 +1483,15 @@ module.exports = {
14831483
}
14841484
}
14851485
}
1486+
},
1487+
public_access_block: {
1488+
type: 'object',
1489+
properties: {
1490+
block_public_acls: { type: 'boolean' },
1491+
ignore_public_acls: { type: 'boolean' },
1492+
block_public_policy: { type: 'boolean' },
1493+
restrict_public_buckets: { type: 'boolean' },
1494+
},
14861495
}
14871496
}
14881497
};

Diff for: ‎src/endpoint/s3/ops/index.js

+3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ exports.delete_bucket_policy = require('./s3_delete_bucket_policy');
1212
exports.delete_bucket_replication = require('./s3_delete_bucket_replication');
1313
exports.delete_bucket_tagging = require('./s3_delete_bucket_tagging');
1414
exports.delete_bucket_website = require('./s3_delete_bucket_website');
15+
exports.delete_bucket_public_access_block = require('./s3_delete_bucket_public_access_block');
1516
exports.delete_object = require('./s3_delete_object');
1617
exports.delete_object_tagging = require('./s3_delete_object_tagging');
1718
exports.delete_object_uploadId = require('./s3_delete_object_uploadId');
@@ -37,6 +38,7 @@ exports.get_bucket_uploads = require('./s3_get_bucket_uploads');
3738
exports.get_bucket_versioning = require('./s3_get_bucket_versioning');
3839
exports.get_bucket_versions = require('./s3_get_bucket_versions');
3940
exports.get_bucket_website = require('./s3_get_bucket_website');
41+
exports.get_bucket_public_access_block = require('./s3_get_bucket_public_access_block');
4042
exports.get_object = require('./s3_get_object');
4143
exports.get_object_acl = require('./s3_get_object_acl');
4244
exports.get_object_attributes = require('./s3_get_object_attributes');
@@ -71,6 +73,7 @@ exports.put_bucket_requestPayment = require('./s3_put_bucket_requestPayment');
7173
exports.put_bucket_tagging = require('./s3_put_bucket_tagging');
7274
exports.put_bucket_versioning = require('./s3_put_bucket_versioning');
7375
exports.put_bucket_website = require('./s3_put_bucket_website');
76+
exports.put_bucket_public_access_block = require('./s3_put_bucket_public_access_block');
7477
exports.put_object = require('./s3_put_object');
7578
exports.put_object_acl = require('./s3_put_object_acl');
7679
exports.put_object_legal_hold = require('./s3_put_object_legal_hold');

Diff for: ‎src/endpoint/s3/ops/s3_delete_bucket_public_access_block.js

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/* Copyright (C) 2024 NooBaa */
2+
'use strict';
3+
4+
/**
5+
* https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeletePublicAccessBlock.html
6+
*/
7+
async function delete_public_access_block(req) {
8+
await req.object_sdk.delete_public_access_block({ name: req.params.bucket });
9+
}
10+
11+
module.exports = {
12+
handler: delete_public_access_block,
13+
body: {
14+
type: 'empty',
15+
},
16+
reply: {
17+
type: 'empty',
18+
},
19+
};
20+

Diff for: ‎src/endpoint/s3/ops/s3_get_bucket_public_access_block.js

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/* Copyright (C) 2024 NooBaa */
2+
'use strict';
3+
4+
/**
5+
* https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetPublicAccessBlock.html
6+
*/
7+
async function get_public_access_block(req) {
8+
const reply = await req.object_sdk.get_public_access_block({ name: req.params.bucket });
9+
if (!reply.public_access_block) {
10+
return {
11+
PublicAccessBlockConfiguration: {
12+
BlockPublicAcls: false,
13+
IgnorePublicAcls: false,
14+
BlockPublicPolicy: false,
15+
RestrictPublicBuckets: false,
16+
}
17+
};
18+
}
19+
20+
return {
21+
PublicAccessBlockConfiguration: {
22+
BlockPublicAcls: Boolean(reply.public_access_block.block_public_acls),
23+
IgnorePublicAcls: Boolean(reply.public_access_block.ignore_public_acls),
24+
BlockPublicPolicy: Boolean(reply.public_access_block.block_public_policy),
25+
RestrictPublicBuckets: Boolean(reply.public_access_block.restrict_public_buckets),
26+
}
27+
};
28+
}
29+
30+
module.exports = {
31+
handler: get_public_access_block,
32+
body: {
33+
type: 'empty'
34+
},
35+
reply: {
36+
type: 'xml',
37+
},
38+
};
39+

Diff for: ‎src/endpoint/s3/ops/s3_put_bucket_public_access_block.js

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/* Copyright (C) 2024 NooBaa */
2+
'use strict';
3+
4+
const s3_utils = require('../s3_utils');
5+
6+
/**
7+
* https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutPublicAccessBlock.html
8+
* @param {nb.S3Request} req
9+
* @param {nb.S3Response} res
10+
*/
11+
async function put_public_access_block(req, res) {
12+
const public_access_block = s3_utils.parse_body_public_access_block(req);
13+
await req.object_sdk.put_public_access_block({ name: req.params.bucket, public_access_block });
14+
}
15+
16+
module.exports = {
17+
handler: put_public_access_block,
18+
body: {
19+
type: 'xml',
20+
},
21+
reply: {
22+
type: 'empty',
23+
},
24+
};
25+

Diff for: ‎src/endpoint/s3/s3_bucket_policy_utils.js

+46-8
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ const OP_NAME_TO_ACTION = Object.freeze({
1717
delete_bucket_replication: { regular: "s3:PutReplicationConfiguration" },
1818
delete_bucket_tagging: { regular: "s3:PutBucketTagging" },
1919
delete_bucket_website: { regular: "s3:DeleteBucketWebsite" },
20+
delete_bucket_public_access_block: { regular: "s3:PutBucketPublicAccessBlock" },
2021
delete_bucket: { regular: "s3:DeleteBucket" },
2122
delete_object_tagging: { regular: "s3:DeleteObjectTagging", versioned: "s3:DeleteObjectVersionTagging" },
2223
delete_object_uploadId: { regular: "s3:AbortMultipartUpload" },
@@ -43,6 +44,7 @@ const OP_NAME_TO_ACTION = Object.freeze({
4344
get_bucket_versions: { regular: "s3:ListBucketVersions" },
4445
get_bucket_website: { regular: "s3:GetBucketWebsite" },
4546
get_bucket_object_lock: { regular: "s3:GetBucketObjectLockConfiguration" },
47+
get_bucket_public_access_block: { regular: "s3:GetBucketPublicAccessBlock" },
4648
get_bucket: { regular: "s3:ListBucket" },
4749
get_object_acl: { regular: "s3:GetObjectAcl" },
4850
get_object_attributes: { regular: ["s3:GetObject", "s3:GetObjectAttributes"], versioned: ["s3:GetObjectVersion", "s3:GetObjectVersionAttributes"] }, // Notice - special case
@@ -80,6 +82,7 @@ const OP_NAME_TO_ACTION = Object.freeze({
8082
put_bucket_versioning: { regular: "s3:PutBucketVersioning" },
8183
put_bucket_website: { regular: "s3:PutBucketWebsite" },
8284
put_bucket_object_lock: { regular: "s3:PutBucketObjectLockConfiguration" },
85+
put_bucket_public_access_block: { regular: "s3:PutBucketPublicAccessBlock" },
8386
put_bucket: { regular: "s3:CreateBucket" },
8487
put_object_acl: { regular: "s3:PutObjectAcl" },
8588
put_object_tagging: { regular: "s3:PutObjectTagging", versioned: "s3:PutObjectVersionTagging" },
@@ -146,18 +149,20 @@ async function _is_object_version_fit(req, predicate, value) {
146149
return res;
147150
}
148151

149-
async function has_bucket_policy_permission(policy, account, method, arn_path, req) {
152+
async function has_bucket_policy_permission(policy, account, method, arn_path, req, disallow_public_access = false) {
150153
const [allow_statements, deny_statements] = _.partition(policy.Statement, statement => statement.Effect === 'Allow');
151154

152155
// the case where the permission is an array started in op get_object_attributes
153156
const method_arr = Array.isArray(method) ? method : [method];
154157

155158
// look for explicit denies
156-
const res_arr_deny = await is_statement_fit_of_method_array(deny_statements, account, method_arr, arn_path, req);
159+
const res_arr_deny = await is_statement_fit_of_method_array(
160+
deny_statements, account, method_arr, arn_path, req); // No need to disallow in "DENY"
157161
if (res_arr_deny.every(item => item)) return 'DENY';
158162

159163
// look for explicit allows
160-
const res_arr_allow = await is_statement_fit_of_method_array(allow_statements, account, method_arr, arn_path, req);
164+
const res_arr_allow = await is_statement_fit_of_method_array(
165+
allow_statements, account, method_arr, arn_path, req, disallow_public_access);
161166
if (res_arr_allow.every(item => item)) return 'ALLOW';
162167

163168
// implicit deny
@@ -177,14 +182,19 @@ function _is_action_fit(method, statement) {
177182
return statement.Action ? action_fit : !action_fit;
178183
}
179184

180-
function _is_principal_fit(account, statement) {
185+
function _is_principal_fit(account, statement, ignore_public_principal = false) {
181186
let statement_principal = statement.Principal || statement.NotPrincipal;
182187

183188
let principal_fit = false;
184189
statement_principal = statement_principal.AWS ? statement_principal.AWS : statement_principal;
185190
for (const principal of _.flatten([statement_principal])) {
186191
dbg.log1('bucket_policy: ', statement.Principal ? 'Principal' : 'NotPrincipal', ' fit?', principal, account);
187192
if ((principal.unwrap() === '*') || (principal.unwrap() === account)) {
193+
if (ignore_public_principal && principal.unwrap() === '*' && statement.Principal) {
194+
// Ignore the "fit" if ignore_public_principal is requested
195+
continue;
196+
}
197+
188198
principal_fit = true;
189199
break;
190200
}
@@ -207,15 +217,15 @@ function _is_resource_fit(arn_path, statement) {
207217
return statement.Resource ? resource_fit : !resource_fit;
208218
}
209219

210-
async function is_statement_fit_of_method_array(statements, account, method_arr, arn_path, req) {
220+
async function is_statement_fit_of_method_array(statements, account, method_arr, arn_path, req, disallow_public_access = false) {
211221
return Promise.all(method_arr.map(method_permission =>
212-
_is_statements_fit(statements, account, method_permission, arn_path, req)));
222+
_is_statements_fit(statements, account, method_permission, arn_path, req, disallow_public_access)));
213223
}
214224

215-
async function _is_statements_fit(statements, account, method, arn_path, req) {
225+
async function _is_statements_fit(statements, account, method, arn_path, req, disallow_public_access = false) {
216226
for (const statement of statements) {
217227
const action_fit = _is_action_fit(method, statement);
218-
const principal_fit = _is_principal_fit(account, statement);
228+
const principal_fit = _is_principal_fit(account, statement, disallow_public_access);
219229
const resource_fit = _is_resource_fit(arn_path, statement);
220230
const condition_fit = await _is_condition_fit(statement, req, method);
221231

@@ -304,6 +314,34 @@ async function validate_s3_policy(policy, bucket_name, get_account_handler) {
304314
}
305315
}
306316

317+
/**
318+
* allows_public_access returns true if a policy will allow public access
319+
* to a resource
320+
*
321+
* NOTE: It assumes that the given policy has already been validated
322+
* @param {*} policy
323+
* @returns {boolean}
324+
*/
325+
function allows_public_access(policy) {
326+
for (const statement of policy.Statement) {
327+
if (statement.Effect === 'Deny') continue;
328+
329+
const statement_principal = statement.Principal;
330+
if (statement_principal.AWS) {
331+
for (const principal of _.flatten([statement_principal.AWS])) {
332+
if (typeof principal === 'string' ? principal === '*' : principal.unwrap() === '*') {
333+
return true;
334+
}
335+
}
336+
} else if (typeof statement_principal === 'string' ? statement_principal === '*' : statement_principal.unwrap() === '*') {
337+
return true;
338+
}
339+
}
340+
341+
return false;
342+
}
343+
307344
exports.OP_NAME_TO_ACTION = OP_NAME_TO_ACTION;
308345
exports.has_bucket_policy_permission = has_bucket_policy_permission;
309346
exports.validate_s3_policy = validate_s3_policy;
347+
exports.allows_public_access = allows_public_access;

Diff for: ‎src/endpoint/s3/s3_rest.js

+7-3
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ const BUCKET_SUB_RESOURCES = Object.freeze({
4545
'encryption': 'encryption',
4646
'object-lock': 'object_lock',
4747
'legal-hold': 'legal_hold',
48-
'retention': 'retention'
48+
'retention': 'retention',
49+
'publicAccessBlock': 'public_access_block'
4950
});
5051

5152
const OBJECT_SUB_RESOURCES = Object.freeze({
@@ -281,20 +282,23 @@ async function authorize_request_policy(req) {
281282
// in case we have bucket policy
282283
let permission_by_id;
283284
let permission_by_name;
285+
286+
const public_access_block_cfg = await req.object_sdk.get_public_access_block({ name: req.params.bucket });
284287
// In NC, we allow principal to be:
285288
// 1. account name (for backwards compatibility)
286289
// 2. account id
287290
// we start the permission check on account identifier intentionally
288291
if (account_identifier_id) {
289292
permission_by_id = await s3_bucket_policy_utils.has_bucket_policy_permission(
290-
s3_policy, account_identifier_id, method, arn_path, req);
293+
s3_policy, account_identifier_id, method, arn_path, req, public_access_block_cfg?.public_access_block?.restrict_public_buckets);
291294
dbg.log3('authorize_request_policy: permission_by_id', permission_by_id);
292295
}
293296
if (permission_by_id === "DENY") throw new S3Error(S3Error.AccessDenied);
294297

295298
if ((!account_identifier_id || permission_by_id !== "DENY") && account.owner === undefined) {
296299
permission_by_name = await s3_bucket_policy_utils.has_bucket_policy_permission(
297-
s3_policy, account_identifier_name, method, arn_path, req);
300+
s3_policy, account_identifier_name, method, arn_path, req, public_access_block_cfg?.public_access_block?.restrict_public_buckets
301+
);
298302
dbg.log3('authorize_request_policy: permission_by_name', permission_by_name);
299303
}
300304
if (permission_by_name === "DENY") throw new S3Error(S3Error.AccessDenied);

Diff for: ‎src/endpoint/s3/s3_utils.js

+20
Original file line numberDiff line numberDiff line change
@@ -811,6 +811,25 @@ function verify_string_byte_length(key, max_length) {
811811
return Buffer.byteLength(key, 'utf8') <= max_length;
812812
}
813813

814+
function parse_body_public_access_block(req) {
815+
const parsed = {};
816+
817+
const access_cfg = req.body?.PublicAccessBlockConfiguration;
818+
if (!access_cfg) throw new S3Error(S3Error.MalformedXML);
819+
820+
if (access_cfg.BlockPublicAcls || access_cfg.IgnorePublicAcls) {
821+
throw new S3Error(S3Error.AccessControlListNotSupported);
822+
}
823+
if (access_cfg.BlockPublicPolicy) {
824+
parsed.block_public_policy = access_cfg.BlockPublicPolicy?.[0].toLowerCase?.() === 'true';
825+
}
826+
if (access_cfg.RestrictPublicBuckets) {
827+
parsed.restrict_public_buckets = access_cfg.RestrictPublicBuckets?.[0].toLowerCase?.() === 'true';
828+
}
829+
830+
return parsed;
831+
}
832+
814833
exports.STORAGE_CLASS_STANDARD = STORAGE_CLASS_STANDARD;
815834
exports.STORAGE_CLASS_GLACIER = STORAGE_CLASS_GLACIER;
816835
exports.STORAGE_CLASS_GLACIER_IR = STORAGE_CLASS_GLACIER_IR;
@@ -855,5 +874,6 @@ exports.cont_tok_to_key_marker = cont_tok_to_key_marker;
855874
exports.key_marker_to_cont_tok = key_marker_to_cont_tok;
856875
exports.parse_sse_c = parse_sse_c;
857876
exports.verify_string_byte_length = verify_string_byte_length;
877+
exports.parse_body_public_access_block = parse_body_public_access_block;
858878
exports.OBJECT_ATTRIBUTES = OBJECT_ATTRIBUTES;
859879
exports.OBJECT_ATTRIBUTES_UNSUPPORTED = OBJECT_ATTRIBUTES_UNSUPPORTED;

0 commit comments

Comments
 (0)