indigo-iam / wlcg-jwt-compliance-tests

Prototype WLCG TPC testsuite using JWT authN/Z
1 stars 11 forks source link

Path authorization tests fail if SE does not use path `/` #34

Closed olifre closed 2 years ago

olifre commented 2 years ago

Trying to get all tests with the XRootD redirector setup at UBonn "green", I found the only remaining issue is about path authorization tests.

The existing tests right now use a claim such as:

storage.read:/wlcg-jwt-compliance

to test. However, an endpoint may have a path already, such as:

https://xrootd.physik.uni-bonn.de:1094/cephfs/grid/wlcg

So a matching claim would be:

storage.read:/cephfs/grid/wlcg/wlcg-jwt-compliance

Trying with such a claim against the endpoint works as expected.

federicaagostini commented 2 years ago

Hi,

also in StoRM WebDAV we have an endpoint (/wlcg) with a path that already exists

$ gfal-ls davs://xfer.cr.cnaf.infn.it:8443/wlcg/
wlcg-jwt-compliance                   
tpc-pull-8c776fc3-984d-4932-8767-a6df1e8481a3
wlcgdoma
...

and when we specify that we want to read recursively just in a subpath of that endpoint (/wlcg/wlcg-jwt-compliance) we use just the storage.read:/wlcg-jwt-compliance scope

$ export BEARER_TOKEN=$(oidc-token -s storage.read:/wlcg-jwt-compliance wlcg)
$ gfal-ls davs://xfer.cr.cnaf.infn.it:8443/wlcg/
gfal-ls error: 1 (Operation not permitted) - HTTP 403 : Permission refused 
$ gfal-ls davs://xfer.cr.cnaf.infn.it:8443/wlcg/wlcg-jwt-compliance
63344036-3ac3-43c8-94bc-85400df22c77
d6cecf3a-6897-4bf8-870a-b7b0189ed233
6a605d25-de83-44f8-b7f9-fda4fc346293
...

So, is it possible to configure the XrootD authz in the same way as StoRM WebDAV does?

federicaagostini commented 2 years ago

Just to add more informations if it can be of any help in the debug: the compliance testsuit logic is based on checking permissions on subpaths that do not exist and expects to receive a 404 Not found HTTP response code.

For instance

$ export BEARER_TOKEN=$(oidc-token -s storage.read:/ wlcg)
$ gfal-ls davs://xrootd.physik.uni-bonn.de:1094/cephfs/grid/wlcg/path-which-does-not-exists
gfal-ls error: 2 (No such file or directory) - Result HTTP 404 : File not found  after 1 attempts

this would result in a green test. But I see now that if we want to check the content of the resource I get a 403 Forbidden HTTP response code using the same token

$ gfal-ls davs://xrootd.physik.uni-bonn.de:1094/cephfs/grid/wlcg
gfal-ls error: 1 (Operation not permitted) - HTTP 403 : Permission refused

This looks strange to me. Anyway, if we want to reproduce the "Path authorization enforced" tests by hand, you are right and I cannot read into the /cephfs/grid/wlcg/wlcg-jwt-compliance path

$ export BEARER_TOKEN=$(oidc-token -s storage.read:/wlcg-jwt-compliance wlcg)
$ gfal-ls davs://xrootd.physik.uni-bonn.de:1094/cephfs/grid/wlcg/wlcg-jwt-compliance/path-which-does-not-exists
gfal-ls error: 1 (Operation not permitted) - HTTP 403 : Permission refused

while I have the same behavior as above if I use the full path in the token

export BEARER_TOKEN=$(oidc-token -s storage.read:/cephfs/grid/wlcg/wlcg-jwt-compliance wlcg)
$ gfal-ls davs://xrootd.physik.uni-bonn.de:1094/cephfs/grid/wlcg/wlcg-jwt-compliance/path-which-does-not-exists
gfal-ls error: 2 (No such file or directory) - Result HTTP 404 : File not found  after 1 attempts
$ gfal-ls davs://xrootd.physik.uni-bonn.de:1094/cephfs/grid/wlcg/wlcg-jwt-compliance                
gfal-ls error: 1 (Operation not permitted) - HTTP 403 : Permission refused
olifre commented 2 years ago

[...] This looks strange to me.

I can confirm, and also reproduce with curl. I'll have a deeper look in a local debug environment later.

So, is it possible to configure the XrootD authz in the same way as StoRM WebDAV does?

I had a deeper look, this does indeed seem to be possible at the token plugin configuration level. Here, I can set a "base path" which is then assumed to be part of the directory in the storage claim. I will reconfigure all nodes this way now, so we should see the effect in the tests tomorrow.

A problem / feature with this configuration is that this is done "by issuer", i.e. it disallows granting access to tokens issued by the WLCG issuer to any paths outside the given base path. In a larger picture, this may make things harder where one issuer serves several organizations, which then can't have their base path since the base path is couple to the issuer.

A notable example from our current configuration is that the path:

/cephfs/grid/srr

contains the SRR JSON, accessible by all WLCG VOs. After adapting the base path for the WLCG VO, it is not accessible via WLCG tokens with storage claims anymore, since it is outside of the "wlcg" path. Or rephrased: Things can't be shared between different issuers.

Let's see if this fixes the tests (my change will become active in ~30 minutes in case you want to test manually before that ;-) ).

olifre commented 2 years ago

[...] This looks strange to me.

I can confirm, and also reproduce with curl. I'll have a deeper look in a local debug environment later.

I believe this is caused by: https://github.com/xrootd/xrootd-scitokens/blob/1f1881fc61e5ca3cfe4a3b5c0aa82d1514937fbf/src/XrdAccSciTokens.cc#L650-L652 translating storage.read into read and stat, but not readdir (which is separate in XRootD). I will confirm this assumption later this week in a debug build of XRootD and if this is true submit a PR. Thanks for pointing out this inconsistency, @federicaagostini !

federicaagostini commented 2 years ago

Let's see if this fixes the tests

"Path authorization" tests are green now (see here); it worked, thank you!
There is now only one test failing: "storage.create does not allow overwriting files".

olifre commented 2 years ago

Thanks for re-running the tests, this looks great indeed — with only one failing test remaining, it's the greenest XRootD in the tests for sure (and being a redirector-setup with multiple DTNs and shared FS, already a complex one) :+1:.

There is now only one test failing: "storage.create does not allow overwriting files".

Indeed. I believe that is (currently) by design, basically in the same place I linked before, the auth mapping is done: https://github.com/xrootd/xrootd-scitokens/blob/1f1881fc61e5ca3cfe4a3b5c0aa82d1514937fbf/src/XrdAccSciTokens.cc#L650-L662 write at this place corresponds to "anything which can write", i.e. storage.create and storage.modify are currently all mapped to the same actual privileges in XRootD unless I miss something.

Since the underlying library exposes the more detailed information already, this should be easy to change. I'll give this a go along with the readdir fix and open PRs, and link them here.

olifre commented 2 years ago

All (known) issues are now fixed with XRootD 5.5.0, which we have upgraded to last night :-).

This means:

The JWT compliance test run from today shows 100 % "green" for [se-ubonn-xrootd](https://ci.cloud.cnaf.infn.it/job/wlcg-jwt-compliance-tests/job/master/565/artifact/reports/reports/latest/joint-report.html#tags?se-ubonn-xrootd), so we can close this issue as solved. Many thanks! :+1: