Skip to content

Commit

Permalink
update for disable access to non-default bucket - issue #138
Browse files Browse the repository at this point in the history
  • Loading branch information
jreadey committed Sep 27, 2022
1 parent fc2e673 commit f6599be
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 5 deletions.
2 changes: 2 additions & 0 deletions admin/config/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ domain_req_max_objects_limit: 500 # maximum number of objects to return in GET d
dn_max_retries: 7 # number of time to retry DN requests
dn_retry_backoff_exp: 0.1 # backoff factor for retries
xss_protection: "1; mode=block" # Include in response headers if set
allow_any_bucket_read: true # enable reads to buckets other than default bucket
allow_any_bucket_write: true # enable writes to buckets other than default bucket
# DEPRECATED - the remaining config values are not used in currently but kept for backward compatibility with older container images
aws_lambda_chunkread_function: null # name of aws lambda function for chunk reading
aws_lambda_threshold: 4 # number of chunks per node per request to reach before using lambda
Expand Down
5 changes: 4 additions & 1 deletion docs/authorization.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,4 +160,7 @@ ACL Inheritance
When a new domain or folder is created, a user ACL will be automatically created that gives the requesting user full control
over that resource. Additionally, any other ACLs (user, group, or default) defined in the parent folder will be copied to the ACLs of the new resource (unless that ACL does not authorize any action).


Bucket Access
-------------

By default HSDS requests can be used to read or write to any bucket (or Azure Container, or Posix directory under ROOT_DIR) that the service has access to. If this is not desired, set the "allow_any_bucket_write" config to false to disable non-default bucket writes, and/or "allow_any_bucket_read" config to false to disable non-default bucket reads. When a request to read or write to a non-allowed bucket is received, a 403 - Forbidden error will be returned.
2 changes: 2 additions & 0 deletions hsds/basenode.py
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,8 @@ def baseInit(node_type):
else:
log.info("no default bucket defined")
app["bucket_name"] = bucket_name
app["allow_any_bucket_read"] = config.get("allow_any_bucket_read", default=True)
app["allow_any_bucket_write"] = config.get("allow_any_bucket_write", default=True)
app["dn_urls"] = []
app["dn_ids"] = [] # node ids for each dn_url

Expand Down
32 changes: 29 additions & 3 deletions hsds/domain_sn.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
from .util.boolparser import BooleanParser
from .util.globparser import globmatch
from .servicenode_lib import getDomainJson, getObjectJson, getObjectIdByPath
from .servicenode_lib import getRootInfo
from .servicenode_lib import getRootInfo, checkBucketAccess
from .basenode import getVersion
from . import hsds_logger as log
from . import config
Expand Down Expand Up @@ -724,6 +724,8 @@ async def GET_Domain(request):
msg = "Bucket not provided"
log.warn(msg)
raise HTTPBadRequest(reason=msg)
if bucket:
checkBucketAccess(app, bucket)

verbose = False
if "verbose" in params and params["verbose"]:
Expand Down Expand Up @@ -922,6 +924,8 @@ async def PUT_Domain(request):
bucket = getBucketForDomain(domain)

log.info(f"PUT domain: {domain}, bucket: {bucket}")
if bucket:
checkBucketAccess(app, bucket, action="write")

body = None
if request.has_body:
Expand Down Expand Up @@ -1221,14 +1225,17 @@ async def DELETE_Domain(request):
except ValueError:
log.warn(f"Invalid domain: {domain}")
raise HTTPBadRequest(reason="Invalid domain name")
bucket = getBucketForDomain(domain)
log.debug(f"GET_Domain domain: {domain}")
log.debug(f"DELETE_Domain domain: {domain}")

if not domain:
msg = "No domain given"
log.warn(msg)
raise HTTPBadRequest(reason=msg)

bucket = getBucketForDomain(domain)
if bucket:
checkBucketAccess(app, bucket, action="delete")

log.info(f"meta_only domain delete: {meta_only}")
if meta_only:
# remove from domain cache if present
Expand Down Expand Up @@ -1340,6 +1347,11 @@ async def GET_ACL(request):
log.warn(msg)
raise HTTPBadRequest(reason=msg)

bucket = getBucketForDomain(domain)

if bucket:
checkBucketAccess(app, bucket)

# use reload to get authoritative domain json
try:
domain_json = await getDomainJson(app, domain, reload=True)
Expand Down Expand Up @@ -1420,6 +1432,10 @@ async def GET_ACLs(request):
log.warn(msg)
raise HTTPBadRequest(reason=msg)

bucket = getBucketForDomain(domain)
if bucket:
checkBucketAccess(app, bucket)

# use reload to get authoritative domain json
try:
domain_json = await getDomainJson(app, domain, reload=True)
Expand Down Expand Up @@ -1510,6 +1526,10 @@ async def PUT_ACL(request):
log.warn(msg)
raise HTTPBadRequest(reason=msg)

bucket = getBucketForDomain(domain)
if bucket:
checkBucketAccess(app, bucket, action="write")

# don't use app["domain_cache"] if a direct domain request is made
# as opposed to an implicit request as with other operations, query
# the domain from the authoritative source (the dn node)
Expand Down Expand Up @@ -1549,6 +1569,8 @@ async def GET_Datasets(request):
bucket = getBucketForDomain(domain)
if not bucket:
bucket = config.get("bucket_name")
else:
checkBucketAccess(app, bucket)

# verify the domain
try:
Expand Down Expand Up @@ -1641,6 +1663,8 @@ async def GET_Groups(request):
bucket = getBucketForDomain(domain)
if not bucket:
bucket = config.get("bucket_name")
else:
checkBucketAccess(app, bucket)

# use reload to get authoritative domain json
try:
Expand Down Expand Up @@ -1728,6 +1752,8 @@ async def GET_Datatypes(request):
bucket = getBucketForDomain(domain)
if not bucket:
bucket = config.get("bucket_name")
else:
checkBucketAccess(app, bucket)

# use reload to get authoritative domain json
try:
Expand Down
21 changes: 21 additions & 0 deletions hsds/servicenode_lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,23 @@ async def getDomainJson(app, domain, reload=False):
return domain_json


def checkBucketAccess(app, bucket, action="read"):
""" if the given bucket is not the default bucket, check
that non-default bucket access is enabled.
Throw 403 error if not allowed """
if bucket and bucket != app["bucket_name"]:
# check that we are allowed to access non-default buckets
if action == "read":
if not app["allow_any_bucket_read"]:
log.warn(f"read access to non-default bucket: {bucket} not enabled")
raise HTTPForbidden()
else:
# write acction
if not app["allow_any_bucket_write"]:
log.warn(f"write access to non-default bucket: {bucket} not enabled")
raise HTTPForbidden()


async def validateAction(app, domain, obj_id, username, action):
"""check that the given object belongs in the domain and that the
requested action (create, read, update, delete, readACL, udpateACL)
Expand All @@ -80,6 +97,10 @@ async def validateAction(app, domain, obj_id, username, action):
msg = f"validateAction(domain={domain}, obj_id={obj_id}, "
msg += f"username={username}, action={action})"
log.info(msg)
bucket = getBucketForDomain(domain)
if bucket:
checkBucketAccess(app, bucket, action=action)

# get domain JSON
domain_json = await getDomainJson(app, domain)
verifyRoot(domain_json)
Expand Down
2 changes: 1 addition & 1 deletion tests/integ/domain_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ def testWithBucket(self):
params = {"bucket": "doesnotexistbucket47839293433"}
req = helper.getEndpoint() + "/"
rsp = self.session.get(req, headers=headers, params=params)
self.assertEqual(rsp.status_code, 404)
self.assertTrue(rsp.status_code in (403, 404))

def testInvalidBucket(self):
domain = self.base_domain + "/invalid_bucket.h5"
Expand Down

0 comments on commit f6599be

Please sign in to comment.