From ddea54f7a15ee46870725fff4e2769800836f411 Mon Sep 17 00:00:00 2001 From: Karl Czajkowski Date: Thu, 26 Oct 2023 11:00:58 -0700 Subject: [PATCH 1/4] doc and test case updates for desired authz consistency changes These changes make the ACL system more consistent and explainable, correcting several accumulated flaws: 1. `;metadata` reads allowed by per-version `read` ACL 2. namespace reads allowed by per-namespace `read` ACL 3. `;versions` reads allowed by per-object `read` ACL 4. object or namespace `subtree-read` ACL affects versions below 5. namespace `subtree-read` ACL affects objects below 6. namespace `subtree-read` ACL affects namespaces below --- docs/REST-API.md | 10 +++-- test/rest-smoketest.sh | 92 +++++++++++++++++++++++++++++++++++++++--- 2 files changed, 93 insertions(+), 9 deletions(-) diff --git a/docs/REST-API.md b/docs/REST-API.md index 5e92410..6263a3c 100644 --- a/docs/REST-API.md +++ b/docs/REST-API.md @@ -726,7 +726,7 @@ as a document: Host: authority_name Accept: application/json If-None-Match: etag_value - + for which the successful response is: 200 OK @@ -816,18 +816,20 @@ each resource type is: - Namespace - `owner`: lists roles considered to be owners of the namespace. - `create`: lists roles permitted to create new children in the namespace. + - `read`: lists roles permitted to read the list of child names in the namespace. - `subtree-owner`: lists roles considered to be owners of any namespace, object, or object version beneath the namespace. - `subtree-create`: lists roles permitted to create new children of the namespace or of any namespace beneath the namespace. - `subtree-update`: lists roles permitted to update data on any object beneath the namespace. - - `subtree-read`: lists roles permitted to read any object version beneath the namespace. + - `subtree-read`: lists roles permitted to read any object version or list children of any namespace beneath the namespace. - Object - `owner`: lists roles considered to be owners of the object. - `update`: lists roles permitted to update object with new versions. + - `read`: lists roles permitted to list versions of the named object. - `subtree-owner`: lists roles considered to be owners of any object version for the object. - - `subtree-read`: lists roles permitted to read any object version for the object. + - `subtree-read`: lists roles permitted to read any object version content for the object. - Object Version - `owner`: lists roles considered to be owners of the object version. - - `read`: lists roles permitted to read the object version. + - `read`: lists roles permitted to read the object version content. ### Lifecycle and Ownership diff --git a/test/rest-smoketest.sh b/test/rest-smoketest.sh index 726244b..5712370 100644 --- a/test/rest-smoketest.sh +++ b/test/rest-smoketest.sh @@ -36,8 +36,8 @@ EOF exit 1 } -[[ -z $GOAUTH && -z $COOKIES ]] && error -[[ -n $COOKIES && ! -r $COOKIES ]] && error +[[ -z $GOAUTH && -z $COOKIES ]] && error must set COOKIES or GOAUTH env var +[[ -n $COOKIES && ! -r $COOKIES ]] && error COOKIES file must be readable #Supported deployments: amazons3 or filesystem DEPLOYMENT=${DEPLOYMENT:-filesystem} @@ -48,10 +48,15 @@ RUNKEY=smoketest-$RANDOM RESPONSE_HEADERS=/tmp/${RUNKEY}-response-headers RESPONSE_CONTENT=/tmp/${RUNKEY}-response-content TEST_DATA=/tmp/${RUNKEY}-test-data +TEST_ACL_ANON=/tmp/${RUNKEY}-test-acl-anon.json + +cat > ${TEST_ACL_ANON} <&2 - summary=$(mycurl "${request[@]}" ) + if [[ "$auth" = true ]] + then + summary=$(mycurl "${request[@]}" ) + else + summary=$(COOKIES= GOAUTH= mycurl "${request[@]}" ) + fi if [[ "${request[0]}" = '--head' ]] then @@ -335,6 +363,15 @@ dotest "200::text/html*::*" "/ns-${RUNKEY}/foo" -H "Accept: text/html" dotest "200::application/json::*" "/ns-${RUNKEY}/foo?cid=smoke" --head dotest "409::*::*" "/ns-${RUNKEY}/foo?cid=smoke" -X PUT -H "Content-Type: application/json" +# check namespace authz corner cases +dotest_anon "401::*::*" "/ns-${RUNKEY}/foo" +dotest_anon "401::*::*" "/ns-${RUNKEY}/foo/bar" +dotest "204::*::*" "/ns-${RUNKEY}/foo;acl/read" -T ${TEST_ACL_ANON} -H "Content-Type: application/json" +dotest_anon "200::application/json::*" "/ns-${RUNKEY}/foo" +dotest_anon "401::*::*" "/ns-${RUNKEY}/foo/bar" +dotest "204::*::*" "/ns-${RUNKEY}/foo;acl/subtree-read" -T ${TEST_ACL_ANON} -H "Content-Type: application/json" +dotest_anon "200::application/json::*" "/ns-${RUNKEY}/foo/bar" + # test objects md5=$(mymd5sum < $0) sha=$(mysha256sum < $0) @@ -344,6 +381,51 @@ dotest "201::text/uri-list::*" /ns-${RUNKEY}/foo/obj1 -X PUT -T $0 -H "Content-T obj1_vers0="$(cat ${RESPONSE_CONTENT})" obj1_vers0="${obj1_vers0#/hatrac}" +# sanity check object status for owner +dotest "200::*::*" "/ns-${RUNKEY}/foo/obj1" +dotest "200::*::*" "/ns-${RUNKEY}/foo/obj1;versions" +dotest "200::*::*" "/ns-${RUNKEY}/foo/obj1;metadata/" +dotest "200::*::*" "${obj1_vers0}" +dotest "200::*::*" "${obj1_vers0};metadata/" + +# check read authz corner cases +# namespace subtree-read (set above) grants all reads to objects and versions below +dotest_anon "200::*::*" "/ns-${RUNKEY}/foo/obj1" +dotest_anon "200::*::*" "${obj1_vers0}" +dotest_anon "200::*::*" "/ns-${RUNKEY}/foo/obj1;versions" +dotest_anon "200::*::*" "/ns-${RUNKEY}/foo/obj1;metadata/" +dotest_anon "200::*::*" "${obj1_vers0};metadata/" +# without read, all read interfaces are blocked +dotest "204::*::*" "/ns-${RUNKEY}/foo;acl/subtree-read" -X DELETE +dotest_anon "401::*::*" "/ns-${RUNKEY}/foo/obj1" +dotest_anon "401::*::*" "${obj1_vers0}" +dotest_anon "401::*::*" "/ns-${RUNKEY}/foo/obj1;versions" +dotest_anon "401::*::*" "/ns-${RUNKEY}/foo/obj1;metadata/" +dotest_anon "401::*::*" "${obj1_vers0};metadata/" +# obj read grants versions listing but not content access +dotest "204::*::*" "/ns-${RUNKEY}/foo/obj1;acl/read" -T ${TEST_ACL_ANON} -H "Content-Type: application/json" +dotest_anon "401::*::*" "/ns-${RUNKEY}/foo/obj1" +dotest_anon "401::*::*" "${obj1_vers0}" +dotest_anon "200::*::*" "/ns-${RUNKEY}/foo/obj1;versions" +dotest_anon "401::*::*" "/ns-${RUNKEY}/foo/obj1;metadata/" +dotest_anon "401::*::*" "${obj1_vers0};metadata/" +# vers read grants content access +dotest "204::*::*" "/ns-${RUNKEY}/foo/obj1;acl/read" -X DELETE +dotest "204::*::*" "${obj1_vers0};acl/read" -T ${TEST_ACL_ANON} -H "Content-Type: application/json" +dotest_anon "200::*::*" "/ns-${RUNKEY}/foo/obj1" +dotest_anon "200::*::*" "${obj1_vers0}" +dotest_anon "401::*::*" "/ns-${RUNKEY}/foo/obj1;versions" +dotest_anon "200::*::*" "/ns-${RUNKEY}/foo/obj1;metadata/" +dotest_anon "200::*::*" "${obj1_vers0};metadata/" +# obj subtree-read grants content access +dotest "204::*::*" "${obj1_vers0};acl/read" -X DELETE +dotest "204::*::*" "/ns-${RUNKEY}/foo/obj1;acl/subtree-read" -T ${TEST_ACL_ANON} -H "Content-Type: application/json" +dotest_anon "200::*::*" "/ns-${RUNKEY}/foo/obj1" +dotest_anon "200::*::*" "${obj1_vers0}" +dotest_anon "401::*::*" "/ns-${RUNKEY}/foo/obj1;versions" +dotest_anon "200::*::*" "/ns-${RUNKEY}/foo/obj1;metadata/" +dotest_anon "200::*::*" "${obj1_vers0};metadata/" + # metadata on object-version dotest "200::application/json::*" "${obj1_vers0};metadata/" dotest "200::application/json::*" "${obj1_vers0};metadata/?cid=smoke" From 5136ed5096a954e0d9eed6aeb71d5c3a13e4d031 Mon Sep 17 00:00:00 2001 From: Karl Czajkowski Date: Thu, 26 Oct 2023 11:17:08 -0700 Subject: [PATCH 2/4] patch namespace read ACL behaviors - allow `read` ACL to be stored on namespaces - fetch read and ancestor subtree-read ACLs for use in namespace logic --- hatrac/model/directory/pgsql.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hatrac/model/directory/pgsql.py b/hatrac/model/directory/pgsql.py index 3f417cb..63ff37f 100644 --- a/hatrac/model/directory/pgsql.py +++ b/hatrac/model/directory/pgsql.py @@ -241,8 +241,8 @@ def drop_acl_role(self, access, role, client_context): class HatracNamespace (HatracName): """Represent a bound namespace.""" - _acl_names = ['owner', 'create', 'subtree-owner', 'subtree-create', 'subtree-read', 'subtree-update'] - _ancestor_acl_names = ['ancestor_owner', 'ancestor_create'] + _acl_names = ['owner', 'create', 'read', 'subtree-owner', 'subtree-create', 'subtree-read', 'subtree-update'] + _ancestor_acl_names = ['ancestor_owner', 'ancestor_create', 'ancestor_read'] def __init__(self, directory, **args): HatracName.__init__(self, directory, **args) From e8988ff0ff354aee543e51c08dbc7ec1b37af17d Mon Sep 17 00:00:00 2001 From: Karl Czajkowski Date: Thu, 26 Oct 2023 11:19:08 -0700 Subject: [PATCH 3/4] patch object and object-version ACL behaviors - load ancestor subtree-read ACLs for use in object logic - include object subtree-{read,owner} ACLs in version read authz check --- hatrac/model/directory/pgsql.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hatrac/model/directory/pgsql.py b/hatrac/model/directory/pgsql.py index 63ff37f..e8b3ec2 100644 --- a/hatrac/model/directory/pgsql.py +++ b/hatrac/model/directory/pgsql.py @@ -264,7 +264,7 @@ def get_content(self, client_context, get_data=True): class HatracObject (HatracName): """Represent a bound object.""" _acl_names = ['owner', 'update', 'read', 'subtree-owner', 'subtree-read'] - _ancestor_acl_names = ['ancestor_owner', 'ancestor_update'] + _ancestor_acl_names = ['ancestor_owner', 'ancestor_update', 'ancestor_read'] def __init__(self, directory, **args): HatracName.__init__(self, directory, **args) @@ -483,7 +483,7 @@ def _prepare_hatrac_stmts(self): WHERE n.name = $1 AND (NOT n.is_deleted OR NOT $2) ; PREPARE hatrac_version_lookup (int8, text) AS - SELECT v.*, n.name, n.pid, n.ancestors, %(owner_acl)s, %(read_acl)s + SELECT v.*, n.name, n.pid, n.ancestors, n."subtree-owner", n."subtree-read", %(owner_acl)s, %(read_acl)s FROM hatrac.version v JOIN hatrac.name n ON (v.nameid = n.id) WHERE v.nameid = $1 AND v.version = $2 ; @@ -1135,7 +1135,8 @@ def _upload_cancel(self, upload, conn=None, cur=None): self._delete_upload(conn, cur, upload) return lambda : self.storage.cancel_upload(upload.name, upload.job) - @db_wrap(enforce_acl=(2, 4, ['owner', 'read', 'ancestor_owner', 'ancestor_read'])) + # subtree-owner and subtree-read are from the parent object name record + @db_wrap(enforce_acl=(2, 4, ['owner', 'read', 'ancestor_owner', 'ancestor_read', 'subtree-owner', 'subtree-read'])) def get_version_content_range(self, object, objversion, get_slice, client_context, get_data=True, conn=None, cur=None): """Return (nbytes, data_generator) pair for specific version.""" if objversion.is_deleted: From 2c3370852485c0a861744d4ce8cb23623aae494e Mon Sep 17 00:00:00 2001 From: Karl Czajkowski Date: Thu, 26 Oct 2023 11:22:03 -0700 Subject: [PATCH 4/4] patch ;metadata ACL behavior Consider these additional ACLs for read access to ;metadata - version-specific `read` ACL - ancestor namespace `subtree-read` ACL - object-level `subtree-read` ACL This aligns the ;metadata read access privileges with the object/object-version GET/HEAD privileges where the same metadata is returned in response headers already. --- hatrac/core.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/hatrac/core.py b/hatrac/core.py index 9a2d939..b74f8e8 100644 --- a/hatrac/core.py +++ b/hatrac/core.py @@ -246,7 +246,8 @@ def is_object(self): return False def get_content(self, client_context, get_data=True): - self.container.resource.enforce_acl(['owner', 'ancestor_owner'], client_context) + # subtree-read is copied from object to object-version + self.container.resource.enforce_acl(['owner', 'read', 'ancestor_owner', 'ancestor_read', 'subtree-read'], client_context) body = self + '\n' return len(body), Metadata({'content-type': 'text/plain'}), body @@ -255,7 +256,8 @@ def is_object(self): return False def get_content(self, client_context, get_data=True): - self.container.resource.enforce_acl(['owner', 'ancestor_owner'], client_context) + # subtree-read is copied from object to object-version + self.container.resource.enforce_acl(['owner', 'read', 'ancestor_owner', 'ancestor_read', 'subtree-read'], client_context) body = base64.b64encode(self) + b'\n' return len(body), Metadata({'content-type': 'text/plain'}), body @@ -313,11 +315,12 @@ def is_object(self): return False def get_content(self, client_context, get_data=True): - self.resource.enforce_acl(['owner', 'ancestor_owner'], client_context) + # subtree-read is copied from object to object-version + self.resource.enforce_acl(['owner', 'read', 'ancestor_owner', 'ancestor_read', 'subtree-read'], client_context) body = jsonWriter(self.to_http()) + b'\n' nbytes = len(body) return nbytes, Metadata({'content-type': 'application/json'}), body - + def _sql_encoded_val(self, k, v): enc, dec = self._sql_codecs.get(k, (lambda x: x, lambda x: x)) return enc(v)